Skip to content

Conversation

@Apehaenger
Copy link
Collaborator

@Apehaenger Apehaenger commented Sep 25, 2025

  1. Moved container shell prefix to open_mower_ros because that's the location where it probably should go if we don't want a common .bashrc for the normal shell as well as the container-shell.
  2. Run Mosquitto as mosquitto user to suppress some early container bootup warnings.
  3. Bundled Dockge and ttyd images. Takes about to 2 minutes for docker load (on 2nd boot)

Draft, because not tested yet

Summary by CodeRabbit

  • New Features

    • Preloads bundled Docker images on first boot to speed startup; startup units now wait for preload and redundant pre-start image pulls removed.
  • Documentation

    • Clarified README/WHATSNEW: setup steps, credentials, boot/LED behavior, device naming, links, and post-start checks.
  • Chores

    • Removed automatic shell prompt tweak; removed STACK_NAME from env template; run MQTT broker container as dedicated mosquitto user.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a Docker image preloading mechanism and systemd unit; updates Dockge and WebTerminal units to depend on it and removes their image-pull presteps. Adjusts Mosquitto to run as mosquitto:mosquitto. Removes PS1 customization. Removes STACK_NAME from the OpenMower .env. Documentation text updated.

Changes

Cohort / File(s) Summary of changes
Documentation updates
README.md, WHATSNEW.md
Text-only edits: reworded/setup instructions, updated links/references, clarified behaviors and release asset names; WHATSNEW updated WebTerminal link and container image reference.
Docker image preload setup
stage-openmower/30-docker/00-run.sh, stage-openmower/30-docker/00-run-chroot.sh, stage-openmower/30-docker/files/etc/systemd/system/docker-preload-images.service
New scripts and systemd unit to stage /opt/docker-images, reconstruct split .part* archives, gunzip .tar.gz to .tar, and preload images at boot with a oneshot docker-preload-images.service that docker loads and removes tars.
Service units aligned to preload
stage-openmower/32-dockge/files/etc/systemd/system/dockge.service, stage-openmower/35-webterminal/files/etc/systemd/system/webterminal.service
Added Wants=/After= dependency on docker-preload-images.service; removed pre-start image-pull ExecStartPre; Dockge gains restart/backoff and time-sync dependencies.
OpenMower stack config tweaks
stage-openmower/40-openmower/files/opt/stacks/openmower/.env, stage-openmower/40-openmower/files/opt/stacks/openmower/compose.yaml
Removed STACK_NAME from .env; updated comments; set Mosquitto container user: "mosquitto:mosquitto".
Shell profile behavior
stage-openmower/40-openmower/00-run-chroot.sh
Removed automatic PS1 modification block that previously prepended an emoji in bashrc files.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

codex

Poem

I stitched up tarballs, one by one,
Loaded images till the preloads were done.
Dockge naps, web shells spring—hop!
Mosquitto in its tidy top.
Stack names gone; the bunny hums, "Deploy!" 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title “Fix Minor Stuff” is too vague and does not convey any of the specific changes made, such as moving the container shell prefix, running Mosquitto as the mosquitto user, or bundling Dockge and ttyd images. Because it uses a generic phrase that fails to summarize the primary updates, it does not meet the clarity requirements. Please choose a more descriptive title that highlights the key changes, for example “Move container shell prefix to open_mower_ros and run Mosquitto as mosquitto user” or similar to clearly reflect the main updates.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/fix-container-shell-prefix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- add time-sync target dependency to ensure clock synchronization before start
- set start limit interval and burst to prevent excessive restart loops
- implement pre-start command to wait for time sync, avoiding TLS certificate errors
- modify docker compose pull with retry logic for network resilience
@Apehaenger Apehaenger changed the title Container shell prefix moved to open_mower_ros Fix Minor Stuff Sep 26, 2025
  - Implement systemd service to preload bundled Docker images on boot
  - Update dockge and webterminal services to depend on preload service
  - Remove redundant pull commands from service files to optimize startup
@Apehaenger Apehaenger requested a review from Copilot September 26, 2025 21:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates several minor fixes and improvements to the OpenMowerOS build configuration, focusing on container setup optimization and user experience improvements.

  • Moves shell prompt customization from system-wide to ROS-specific context
  • Configures Mosquitto to run with proper user permissions to suppress warnings
  • Bundles Docker images to eliminate initial pull delays during startup

Reviewed Changes

Copilot reviewed 10 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
stage-openmower/40-openmower/files/opt/stacks/openmower/compose.yaml Adds mosquitto user configuration to Docker compose
stage-openmower/40-openmower/files/opt/stacks/openmower/.env Updates ESC comment and removes STACK_NAME variable
stage-openmower/40-openmower/00-run-chroot.sh Removes bash prompt modification for container context
stage-openmower/35-webterminal/files/etc/systemd/system/webterminal.service Updates service dependencies to include preload service
stage-openmower/32-dockge/files/etc/systemd/system/dockge.service Adds restart policies and preload dependency
stage-openmower/30-docker/files/etc/systemd/system/docker-preload-images.service Creates new service for bundled image loading
stage-openmower/30-docker/00-run.sh Implements Docker image bundling and extraction logic
stage-openmower/30-docker/00-run-chroot.sh Enables preload service and creates image directory
WHATSNEW.md Updates documentation links and descriptions
README.md Corrects documentation and improves setup instructions

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Apehaenger and others added 3 commits September 26, 2025 23:29
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix typo

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix typo

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Apehaenger Apehaenger marked this pull request as ready for review September 26, 2025 21:31
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1a93df5 and fdb4efe.

⛔ Files ignored due to path filters (2)
  • .github/img/dockge_01.jpg is excluded by !**/*.jpg
  • stage-openmower/30-docker/files/opt/docker-images/ttyd.tar.gz is excluded by !**/*.gz
📒 Files selected for processing (10)
  • README.md (4 hunks)
  • WHATSNEW.md (2 hunks)
  • stage-openmower/30-docker/00-run-chroot.sh (1 hunks)
  • stage-openmower/30-docker/00-run.sh (1 hunks)
  • stage-openmower/30-docker/files/etc/systemd/system/docker-preload-images.service (1 hunks)
  • stage-openmower/32-dockge/files/etc/systemd/system/dockge.service (1 hunks)
  • stage-openmower/35-webterminal/files/etc/systemd/system/webterminal.service (1 hunks)
  • stage-openmower/40-openmower/00-run-chroot.sh (0 hunks)
  • stage-openmower/40-openmower/files/opt/stacks/openmower/.env (1 hunks)
  • stage-openmower/40-openmower/files/opt/stacks/openmower/compose.yaml (1 hunks)
💤 Files with no reviewable changes (1)
  • stage-openmower/40-openmower/00-run-chroot.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build

@ClemensElflein ClemensElflein merged commit 84d8f7d into main Oct 1, 2025
2 checks passed
Apehaenger added a commit to ClemensElflein/open_mower_ros that referenced this pull request Oct 3, 2025
Moved container shell prefix to here instead of OpenMowerOS. See
ClemensElflein/OpenMowerOS#27

~~Draft, because not yet tested.~~

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Shell prompt now shows a tractor emoji (🚜) when running in a
virtualized environment.
  * Colorized prompt is automatically enabled when supported.

* **Chores**
* Streamlined .bashrc setup in the container by conditionally enabling
force_color_prompt instead of always creating the file.
* Removed legacy STACK_SHELL-based prompt prefix logic for a cleaner,
consistent prompt behavior.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Clemens Elflein <clemens1@familie-elflein.de>
ClemensElflein added a commit to ClemensElflein/open_mower_ros that referenced this pull request Dec 3, 2025
Moved container shell prefix to here instead of OpenMowerOS. See
ClemensElflein/OpenMowerOS#27

~~Draft, because not yet tested.~~

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

* **New Features**
* Shell prompt now shows a tractor emoji (🚜) when running in a
virtualized environment.
  * Colorized prompt is automatically enabled when supported.

* **Chores**
* Streamlined .bashrc setup in the container by conditionally enabling
force_color_prompt instead of always creating the file.
* Removed legacy STACK_SHELL-based prompt prefix logic for a cleaner,
consistent prompt behavior.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Clemens Elflein <clemens1@familie-elflein.de>
@coderabbitai coderabbitai bot mentioned this pull request Jan 3, 2026
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.

3 participants