Conversation
| "-w", | ||
| "--workspace", | ||
| default=".", | ||
| default="/home/bitbots/bitbots_main", |
There was a problem hiding this comment.
Where do we want to put the bitbots_main dir on the robot?
There was a problem hiding this comment.
I would expect bitbots_main to live in the home dir
There was a problem hiding this comment.
Pull request overview
This pull request updates and simplifies the Bit-Bots public documentation, focusing on installation instructions, deployment processes, and general documentation improvements.
Changes:
- Updated installation guide to reflect current pixi-based workflow and removed outdated ROS2-specific instructions
- Reorganized robot deployment documentation, moving Ansible configuration section before deployment steps
- Corrected spelling errors and improved formatting consistency across multiple documentation files
- Removed obsolete style guide XML files for Python and C++ as the project now uses pre-commit hooks
- Updated script documentation and fixed variable naming inconsistencies (e.g., meta_dir → main_dir)
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/bitbots_misc/bitbots_docs/package.xml | Cleaned up XML formatting and removed obsolete documentation metadata |
| src/bitbots_misc/bitbots_docs/docs/manual/tutorials/webots_simulation_testing.rst | Simplified setup instructions and removed redundant SSH key setup steps |
| src/bitbots_misc/bitbots_docs/docs/manual/tutorials/vscode-ros2.rst | Updated to reflect pixi environment instead of ROS sourcing |
| src/bitbots_misc/bitbots_docs/docs/manual/tutorials/lowlevel.rst | Removed GitHub repository link and cleaned up whitespace |
| src/bitbots_misc/bitbots_docs/docs/manual/tutorials/installation.rst | Updated guide title, fixed prerequisites spelling, improved formatting, and updated Ubuntu version references |
| src/bitbots_misc/bitbots_docs/docs/manual/tutorials/configure_and_flash_robot.rst | Reorganized sections, moved Ansible configuration before deployment, updated terminology from "Flash" to "Deploy" |
| src/bitbots_misc/bitbots_docs/docs/manual/tutorials/competition_wifi.rst | Updated ansible repository URL from git.mafiasi.de to GitHub |
| src/bitbots_misc/bitbots_docs/docs/manual/testing/testing.rst | Fixed section header underlines for proper reStructuredText formatting |
| src/bitbots_misc/bitbots_docs/docs/manual/testing/test_robot_hardware.rst | Updated deployment terminology and improved wording |
| src/bitbots_misc/bitbots_docs/docs/manual/testing/test_motion.rst | Fixed spelling of "Alternatively" |
| src/bitbots_misc/bitbots_docs/docs/manual/testing/competition_preparation.rst | Updated "NUC" references to generic "PC" |
| src/bitbots_misc/bitbots_docs/docs/manual/software/coding_style.rst | Replaced detailed style guides with pre-commit hook documentation |
| src/bitbots_misc/bitbots_docs/docs/manual/hardware/mechanics/screws.rst | Added warning note about outdated content |
| src/bitbots_misc/bitbots_docs/docs/index.rst | Updated main page description and corrected Github→GitHub |
| src/bitbots_misc/bitbots_docs/docs/_static/bitbots_python_style.xml | Removed obsolete Python style configuration file |
| src/bitbots_misc/bitbots_docs/docs/_static/bitbots_cpp_style.xml | Removed obsolete C++ style configuration file |
| src/bitbots_misc/bitbots_docs/cmake/enable_bitbots_docs.cmake.in | Fixed spelling of "necessary" and updated comment about catkin→package |
| src/bitbots_misc/bitbots_docs/CMakeLists.txt | Updated comment removing catkin reference |
| scripts/setup.sh | Renamed variables from meta_dir to main_dir, added HTTPS fallback for git clone, improved basler camera handling |
| scripts/make_basler.sh | Added Ubuntu version check to ensure compatibility with Pylon driver package |
| scripts/deploy/deploy_robots.py | Changed default workspace path to absolute path |
| scripts/README.md | Updated task numbering and descriptions |
| README.md | Simplified installation instructions with reference to documentation |
Comments suppressed due to low confidence (2)
src/bitbots_misc/bitbots_docs/docs/manual/tutorials/installation.rst:9
- Corrected spelling of 'Prerequisites' from 'Prerequirements'.
src/bitbots_misc/bitbots_docs/docs/manual/tutorials/installation.rst:43 - The URL protocol should be HTTPS instead of HTTP for security. Changed from 'http://doku.bit-bots.de' to 'https://doku.bit-bots.de'.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/bitbots_misc/bitbots_docs/docs/manual/tutorials/configure_and_flash_robot.rst
Outdated
Show resolved
Hide resolved
src/bitbots_misc/bitbots_docs/docs/manual/tutorials/lowlevel.rst
Outdated
Show resolved
Hide resolved
…and_flash_robot.rst Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| git clone "$REPO_URL" | ||
| echo "Cloning repository bitbots_main..." | ||
| # Try to clone via SSH first, fall back to HTTPS if that fails | ||
| if ! git clone "$REPO_URL_SSH"; then |
There was a problem hiding this comment.
I am not sure if I like this implicit fallback. It might cause problems later on if people try to push via ssh.
There was a problem hiding this comment.
I am also not 100% happy with that.
Who is the target user of this script?
- Obviously, we want our members to use this script and we can throw and error message explaining how to setup the SSH key on GitHub and try again, then exit.
- But in general, these kind of scripts are targeted at the general external community, which we also want to support. These people might only "consume" software and do not intend to ever contribute. They might not even have a GitHub account, and demanding them to create an account and upload some weird S s H (?) key just to continue this crashing one-script-to-setup-everything is too much.
Compromise: If SSH clone fails, warn about missing keys and link to guide how to set them up. Then ask user, if they intend to fix this or continue with HTTPS.
src/bitbots_misc/bitbots_docs/docs/manual/tutorials/configure_and_flash_robot.rst
Show resolved
Hide resolved
texhnolyze
left a comment
There was a problem hiding this comment.
Does it make sense to add a tiny paragraph to old docs referencing,
ros2 run ... or colcon build ... commands directly
that all ros/colcon commands should be done in a pixi shell?
Or do you think that's self explanatory?
| check_os_is_required_ubuntu_version () { | ||
| # Check if the OS is ubuntu | ||
| if [[ "$(lsb_release -is)" != "Ubuntu" ]]; then | ||
| echo "This driver package only supports Ubuntu. Please install Ubuntu $REQUIRED_UBUNTU_VERSION and try again." |
There was a problem hiding this comment.
I'm not sure if that is true the dependencies of pylon are literally only:
Depends: libstdc++6
Depends: libc6
which most likely is available on debian as well
|
|
||
| # Check if the ubuntu version is the required one | ||
| if [[ "$(lsb_release -rs)" != "$REQUIRED_UBUNTU_VERSION" ]]; then | ||
| echo "This driver package only supports Ubuntu $REQUIRED_UBUNTU_VERSION. Please install Ubuntu $REQUIRED_UBUNTU_VERSION and try again." |
There was a problem hiding this comment.
This is most definitely not true.
According to the latest pylon readme
it is literally built on ubuntu 18.04
| @@ -7,6 +7,7 @@ set -eEo pipefail | |||
| # Go to the download button and copy the link address. | |||
| PYLON_DOWNLOAD_URL="https://data.bit-bots.de/pylon_7_4_0_14900_linux_x86_64_debs.tar.gz.gpg" | |||
There was a problem hiding this comment.
Pylon 7.4.0 was released in September 2023, the latest available version is from January 2026 version 26.01 (pylon has changed to a yy.mm version scheme).
Should we upgrade at some point (and create an issue for this) or is there anything speaking against that?
|
|
||
| # Append "--packages-skip bitbots_basler_camera" to the build command if setup_host was skipped or failed | ||
| if (( basler_installed )); then | ||
| $HOME/.pixi/bin/pixi run build |
There was a problem hiding this comment.
This will not work with a global pixi install or other potential installation locations (for whatever reason).
I would prefer adding export PATH="$PATH:$HOME/.pixi/bin" to setup_pixi if it was freshly installed (it is automatically added to .zshrc by the script anyway).
Also Indentation
| if (( basler_installed )); then | ||
| $HOME/.pixi/bin/pixi run build | ||
| else | ||
| $HOME/.pixi/bin/pixi run build --packages-skip bitbots_basler_camera |
| done | ||
| } | ||
|
|
||
| setup_pixi() { |
There was a problem hiding this comment.
| setup_pixi() { | |
| if ! type pixi &> /dev/null; then | |
| curl -fsSL https://pixi.sh/install.sh | sh | |
| fi | |
| } |
This prevents unnecessary downloads and overwriting a global pixi install (e.g. on arch) with a local one.
Summary
This updates several sections of our public documentation and simplifies the installation instructions a bit.
Proposed changes
Related issues
Checklist
colcon build