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

Improved new developer experience: documentation & environment setup #34317

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
00cf4a8
control webcam with ENV vars
MikeBusuttil Dec 12, 2024
eff3308
WIP: actual instructions
MikeBusuttil Dec 12, 2024
6f61325
wording
MikeBusuttil Dec 12, 2024
d66aa66
file no longer exists
MikeBusuttil Dec 12, 2024
7576f3e
this is expected behavior, just untested
MikeBusuttil Dec 12, 2024
7f7c1af
more readable
MikeBusuttil Dec 12, 2024
e8e7c52
tested on fresh install
MikeBusuttil Dec 12, 2024
a4c43e9
Merge branch 'master' into dehack-webcam
MikeBusuttil Dec 12, 2024
79950d3
enumerate WSL Issues
MikeBusuttil Dec 12, 2024
1d256b7
better wording
MikeBusuttil Dec 15, 2024
c99f0f0
critical WSL installation steps and packages added
MikeBusuttil Dec 16, 2024
7c45c05
Merge branch 'master' into WSL-gotchas
MikeBusuttil Dec 16, 2024
4121a1b
wording, drivers now included in installer
MikeBusuttil Dec 16, 2024
68f3e84
Merge branch 'master' into dehack-webcam
MikeBusuttil Dec 16, 2024
ba4acc7
Merge branch 'dehack-webcam' into WSL-gotchas
MikeBusuttil Dec 16, 2024
fc4c60b
fix url
MikeBusuttil Dec 16, 2024
84026b4
fix url
MikeBusuttil Dec 16, 2024
9d2dc29
wording tweaks
MikeBusuttil Dec 16, 2024
d2800bf
Merge branch 'dehack-webcam' into WSL-gotchas
MikeBusuttil Dec 16, 2024
d7f394b
add openpilot install step
MikeBusuttil Dec 16, 2024
63e06be
UI jank
MikeBusuttil Dec 16, 2024
113fbb2
UI jank
MikeBusuttil Dec 16, 2024
a1672d1
rerun issues
MikeBusuttil Dec 16, 2024
e10936f
caveats moved to linked wiki, redundant Git LFS text removed
MikeBusuttil Dec 26, 2024
31ad649
Merge branch 'master' into WSL-gotchas
MikeBusuttil Dec 26, 2024
8a92a1f
Merge branch 'master' into WSL-gotchas
MikeBusuttil Dec 26, 2024
963ba65
link to wiki
MikeBusuttil Dec 26, 2024
9816116
remove already-installed package
MikeBusuttil Dec 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,18 @@ Development is coordinated through [Discord](https://discord.comma.ai) and GitHu
* Read about the [development workflow](WORKFLOW.md)
* Join our [Discord](https://discord.comma.ai)
* Docs are at https://docs.comma.ai and https://blog.comma.ai
* Read the [Introduction to Openpilot](https://github.com/commaai/openpilot/wiki/Introduction-to-openpilot)

## What contributions are we looking for?

**openpilot's priorities are [safety](SAFETY.md), stability, quality, and features, in that order.**
openpilot is part of comma's mission to *solve self-driving cars while delivering shippable intermediaries*, and all development is towards that goal.
openpilot is part of comma's mission to *solve self-driving cars while delivering shippable intermediaries*, and all development is towards that goal.

### What gets merged?

The probability of a pull request being merged is a function of its value to the project and the effort it will take us to get it merged.
If a PR offers *some* value but will take lots of time to get merged, it will be closed.
Simple, well-tested bug fixes are the easiest to merge, and new features are the hardest to get merged.
Simple, well-tested bug fixes are the easiest to merge, and new features are the hardest to get merged.

All of these are examples of good PRs:
* typo fix: https://github.com/commaai/openpilot/pull/30678
Expand All @@ -30,7 +31,7 @@ All of these are examples of good PRs:

### What doesn't get merged?

* **style changes**: code is art, and it's up to the author to make it beautiful
* **style changes**: code is art, and it's up to the author to make it beautiful
* **500+ line PRs**: clean it up, break it up into smaller PRs, or both
* **PRs without a clear goal**: every PR must have a singular and clear goal
* **UI design**: we do not have a good review process for this yet
Expand Down Expand Up @@ -63,3 +64,4 @@ A good pull request has all of the following:
* Connect your device to Wi-Fi regularly, so that we can pull data for training better driving models.
* Run the `nightly` branch and report issues. This branch is like `master` but it's built just like a release.
* Annotate images in the [comma10k dataset](https://github.com/commaai/comma10k).
* Update the [wiki](https://github.com/commaai/openpilot/wiki)
22 changes: 3 additions & 19 deletions tools/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@

## System Requirements

openpilot is developed and tested on **Ubuntu 24.04**, which is the primary development target aside from the [supported embedded hardware](https://github.com/commaai/openpilot#running-on-a-dedicated-device-in-a-car).
openpilot is developed and tested on **Ubuntu 24.04** with [modern hardware](https://github.com/commaai/openpilot/wiki/Requirements#hardware), which is the primary development target aside from the [supported embedded hardware](https://comma.ai/shop/comma-3x).

Most of openpilot should work natively on macOS. On Windows you can use WSL for a nearly native Ubuntu experience. Running natively on any other system is not currently recommended and will likely require modifications.
Most of openpilot should work natively on macOS. On Windows you can use WSL2 for a nearly native Ubuntu experience [with some exceptions](https://github.com/commaai/openpilot/wiki/WSL). Running natively on any other system is not currently recommended and will likely require modifications.

## Native setup on Ubuntu 24.04 and macOS

**1. Clone openpilot**

NOTE: This repository uses Git LFS for large files. Ensure you have [Git LFS](https://git-lfs.com/) installed and set up before cloning or working with it.

Either do a partial clone for faster download:
``` bash
git clone --filter=blob:none --recurse-submodules --also-filter-submodules https://github.com/commaai/openpilot.git
Expand All @@ -29,12 +27,6 @@ cd openpilot
tools/op.sh setup
```

**3. Git LFS**

``` bash
git lfs pull
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

op.sh takes care of Git LFS (which is now installed with git on Ubuntu 24.04)

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should replace 1-3 with bash <(curl -fsSL openpilot.comma.ai).

**4. Activate a python shell**

Activate a shell with the Python dependencies installed:
Expand All @@ -48,16 +40,8 @@ source .venv/bin/activate
scons -u -j$(nproc)
```

## WSL on Windows

[Windows Subsystem for Linux (WSL)](https://docs.microsoft.com/en-us/windows/wsl/about) should provide a similar experience to native Ubuntu. [WSL 2](https://docs.microsoft.com/en-us/windows/wsl/compare-versions) specifically has been reported by several users to be a seamless experience.

Follow [these instructions](https://docs.microsoft.com/en-us/windows/wsl/install) to setup the WSL and install the `Ubuntu-24.04` distribution. Once your Ubuntu WSL environment is setup, follow the Linux setup instructions to finish setting up your environment. See [these instructions](https://learn.microsoft.com/en-us/windows/wsl/tutorials/gui-apps) for running GUI apps.

**NOTE**: If you are running WSL and any GUIs are failing (segfaulting or other strange issues) even after following the steps above, you may need to enable software rendering with `LIBGL_ALWAYS_SOFTWARE=1`, e.g. `LIBGL_ALWAYS_SOFTWARE=1 selfdrive/ui/ui`.

Comment on lines -51 to -58
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm neutral on moving the WSL stuff to Wiki, but if we're going to move it, we need to preserve the explicit instruction to use Ubuntu 24.04 as the distribution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, but I think it's pretty explicit both here and on the WSL wiki. Do you think it needs to be made more explicit?

## CTF
Learn about the openpilot ecosystem and tools by playing our [CTF](/tools/CTF.md).
Learn about the openpilot ecosystem and tools by playing our [CTF](/tools/CTF.md) and [reading the wiki](https://github.com/commaai/openpilot/wiki/Introduction-to-openpilot).

## Directory Structure

Expand Down
4 changes: 4 additions & 0 deletions tools/install_ubuntu_dependencies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ function install_ubuntu_common_requirements() {
libbz2-dev \
libeigen3-dev \
libffi-dev \
libfreeimage-dev \
libfreeimage3 \
Comment on lines +44 to +45
Copy link
Contributor Author

Choose a reason for hiding this comment

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

required for rerun

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's leave these out for now, I'm not sure rerun is moving forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok sure. I'll put them in the WSL docs

libglew-dev \
libgles2-mesa-dev \
libglfw3-dev \
Expand All @@ -57,7 +59,9 @@ function install_ubuntu_common_requirements() {
opencl-headers \
ocl-icd-libopencl1 \
ocl-icd-opencl-dev \
pocl-opencl-icd \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

required for OpenCL

Copy link
Collaborator

Choose a reason for hiding this comment

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

Required for webcam only, which is a relatively rare need. I just checked on my machine, it wants to drag in 22 dependencies and 845MB of disk. Let's leave it as manual for webcam users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI it's also required for sim users. You're right, It is hefty, as are the CUDA drivers - but these are important to mention to devs that'll be attempting to run anything that uses models. I'll mention it in the docs instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please be very mindful before adding packages to this list since it makes everything big and slow.

The pocl icd is also very slow since it is a software implementation of opencl, so it is better to mention it as a last resort for people without a driver installed already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's good to know Max, I wasn't aware, I'll remove it from here and add it to some documentation elsewhere

portaudio19-dev \
pulseaudio \
Copy link
Contributor Author

@MikeBusuttil MikeBusuttil Dec 27, 2024

Choose a reason for hiding this comment

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

required for audio on WSL2 (ie. in sumulator)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is a good idea. Native Ubuntu 24.04 is using pipewire instead of pulseaudio; apt wants to remove pipewire when asked to install pulseaudio. This is a big change to user systems that we shouldn't be making. Maybe just add it to the WSL2 wiki.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ya that's fair. I'll try pipewire instead on WSL and make sure that behaves nicely

qttools5-dev-tools \
libqt5svg5-dev \
libqt5serialbus5-dev \
Expand Down
4 changes: 0 additions & 4 deletions tools/webcam/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ That's it!

## Setup openpilot
- Follow [this readme](../README.md) to install and build the requirements
- Install OpenCL Driver
```
sudo apt install pocl-opencl-icd
```

## Connect the hardware
- Connect the road facing camera first, then the driver facing camera
Expand Down
Loading