Skip to content

Add setup script and README for Brother HL-L1222-V Android print server on Raspberry Pi (openSUSE)#29

Open
ib-bsb-br wants to merge 1 commit into
mainfrom
codex/create-tutorial-for-brother-hl-l1222-v-setup-on-raspberry-pi-ed7jsi
Open

Add setup script and README for Brother HL-L1222-V Android print server on Raspberry Pi (openSUSE)#29
ib-bsb-br wants to merge 1 commit into
mainfrom
codex/create-tutorial-for-brother-hl-l1222-v-setup-on-raspberry-pi-ed7jsi

Conversation

@ib-bsb-br

@ib-bsb-br ib-bsb-br commented Apr 29, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Provide an automated, opinionated setup for configuring a Raspberry Pi 4B+ (openSUSE Tumbleweed, ARM64) as a print server for a USB-connected Brother HL-L1222/HL-L1222-V to enable Android printing over Wi‑Fi (IPP/Mopria).
  • Make CUPS, mDNS/Avahi and firewall configuration, printer queue creation, and optional fallback services reproducible and easier to deploy.
  • Offer optional conveniences such as a hotfolder auto-print watcher and a small LAN upload web page to support devices or workflows that cannot use direct IPP printing.

Description

  • Added scripts/setup_hl_l1222_android_print.sh, a comprehensive installer that installs packages, enables services (cups, avahi-daemon, firewalld, optional bluetooth), configures firewall rules, and adjusts CUPS sharing using cupsctl and safe backups of /etc/cups/cupsd.conf.
  • The script detects the USB device URI and selects a print model (prefers brlaser or falls back to the everywhere model or a generic monochrome laser), creates or replaces the CUPS queue named by QUEUE_NAME, and sets sensible defaults and sharing options.
  • Installs a Python hotfolder watcher at /usr/local/lib/hll1222-print/hotfolder_watcher.py and a corresponding systemd service (hll1222-hotfolder.service) that runs as user lp for auto-printing files placed into /var/spool/hll1222-hotfolder.
  • Provides an optional small upload HTTP server (upload_server.py) and systemd unit (hll1222-upload.service) for LAN uploads to the hotfolder, plus a README update (README.md) with usage notes and run instructions.
  • The installer includes CLI options (--device, --model, --enable-bluetooth, --enable-upload, --upload-port, --status) and safety prompts for high-risk operations such as modifying CUPS config or deleting an existing queue.

Testing

  • No automated tests were run for this change.

Codex Task


Open in Devin Review

Summary by Sourcery

Add an automated setup for configuring a Raspberry Pi (openSUSE Tumbleweed) as a CUPS-based print server for a USB-connected Brother HL-L1222/HL-L1222-V to enable Android Wi‑Fi printing via IPP/Mopria, including optional hotfolder and upload services.

New Features:

  • Provide a bash setup script that installs and configures CUPS, Avahi, firewalld, and optional Bluetooth support for a Brother HL-L1222/HL-L1222-V print server on Raspberry Pi.
  • Automatically detect the printer USB device URI and select an appropriate print model, creating or recreating a shared CUPS queue with sensible defaults.
  • Add a Python-based hotfolder auto-print watcher with a dedicated systemd service to print files dropped into a spool directory.
  • Introduce an optional LAN HTTP upload service and systemd unit that lets users upload files to the hotfolder for printing.
  • Document the setup and Android printing workflow in the README, including basic usage and connection instructions.

Enhancements:

  • Harden the setup process with safety prompts, configuration backups, and a status/diagnostics mode to make deployments more reproducible and less error-prone.

Documentation:

  • Replace the placeholder README with usage documentation for the Brother HL-L1222-V Android print server setup script, outlining capabilities, limitations, and run instructions.

Summary by cubic

Adds a one-command setup to turn a Raspberry Pi 4B+ (openSUSE Tumbleweed) into an IPP/Mopria print server for a USB Brother HL‑L1222/HL‑L1222‑V. Automates cups sharing, avahi-daemon discovery, firewalld rules, queue creation, and optional hotfolder/upload helpers.

  • New Features
    • Added scripts/setup_hl_l1222_android_print.sh to install packages, enable cups, avahi-daemon, firewalld (optional bluetooth), open IPP/mDNS, and configure sharing via cupsctl with a backup of /etc/cups/cupsd.conf.
    • Auto-detects USB device URI; selects model preferring printer-driver-brlaser, with fallbacks to everywhere or a generic mono driver; creates/replaces the Brother_HL_L1222_V queue with sane defaults and sharing.
    • Installs a hotfolder watcher (/usr/local/lib/hll1222-print/hotfolder_watcher.py) with hll1222-hotfolder.service (runs as lp) to auto-print files from /var/spool/hll1222-hotfolder.
    • Optional upload HTTP server (/usr/local/lib/hll1222-print/upload_server.py) with hll1222-upload.service on --upload-port (default 8088) for LAN file drops into the hotfolder.
    • CLI flags: --device, --model, --enable-bluetooth, --enable-upload, --upload-port, --status; includes safety prompts before CUPS changes or queue replacement.
    • Updated README.md with run steps and Android Mopria setup.

Written for commit 82f6567. Summary will update on new commits. Review in cubic

@sourcery-ai

sourcery-ai Bot commented Apr 29, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds an opinionated setup script and documentation to configure a Raspberry Pi 4B+ running openSUSE Tumbleweed as a CUPS-based Android print server for a USB-connected Brother HL-L1222-V, including service enablement, firewall rules, printer queue provisioning, and optional hotfolder and upload HTTP services managed via systemd.

Sequence diagram for Android IPP printing via CUPS

sequenceDiagram
  actor AndroidPhone
  participant Avahi
  participant CUPS
  participant USBPrinter

  AndroidPhone->>Avahi: Discover IPP printers via mDNS
  Avahi-->>AndroidPhone: Advertise HL_L1222_V queue
  AndroidPhone->>CUPS: IPP print job (document)
  CUPS-->>AndroidPhone: IPP job accepted
  CUPS->>USBPrinter: Send rasterized job over USB
  USBPrinter-->>CUPS: Print completion
  CUPS-->>AndroidPhone: IPP job completed status
Loading

Sequence diagram for optional upload hotfolder printing path

sequenceDiagram
  actor UserBrowser
  participant UploadServer
  participant HotfolderDir
  participant HotfolderWatcher
  participant CUPS
  participant USBPrinter

  UserBrowser->>UploadServer: HTTP POST /upload (file)
  UploadServer->>UploadServer: Validate size and extension
  UploadServer->>HotfolderDir: Save file into hotfolder
  UploadServer-->>UserBrowser: 200 OK (queued message)

  loop every 3 seconds
    HotfolderWatcher->>HotfolderDir: Scan for new stable files
  end
  HotfolderWatcher->>CUPS: lp -d QUEUE_NAME file
  CUPS-->>HotfolderWatcher: Job accepted
  CUPS->>USBPrinter: Send job over USB
  USBPrinter-->>CUPS: Print completion
  HotfolderWatcher->>HotfolderDir: Move file to printed or failed
Loading

Flowchart for setup_hl_l1222_android_print.sh installer logic

flowchart TD
  A[Start setup_hl_l1222_android_print_sh] --> B[Parse CLI arguments
  queue, device, model,
  bluetooth, upload, status]
  B --> C{STATUS_ONLY?}
  C -- yes --> D[Run status_report
  uname, lpinfo, lpstat]
  D --> Z[Exit]
  C -- no --> E[require_command for
  sudo, zypper, lpinfo,
  lpadmin, lpstat]
  E --> F[install_dependencies
  cups, avahi, firewalld,
  optional brlaser, bluetooth]
  F --> G[enable_services
  cups, avahi, firewalld,
  optional bluetooth]
  G --> H[configure_firewall
  allow ipp, mdns,
  optional upload_port]
  H --> I[configure_cups_sharing
  backup cupsd.conf,
  cupsctl --share-printers,
  enable WebInterface,
  Browsing, dnssd,
  restart cups]
  I --> J[detect_usb_uri
  via lpinfo -v]
  J --> K[select_model
  prefer brlaser/Brother,
  else everywhere,
  else generic mono]
  K --> L{Queue exists?}
  L -- yes --> M[confirm_high_risk
  delete existing queue]
  M --> N[lpadmin -x QUEUE_NAME]
  L -- no --> N
  N --> O[Create queue
  lpadmin -p QUEUE_NAME
  -v DEVICE_URI -m MODEL_NAME]
  O --> P[Set defaults and sharing
  A4, one-sided,
  printer-is-shared]
  P --> Q[Enable and accept queue
  cupsenable, cupsaccept,
  lpoptions -d]
  Q --> R[Restart cups]
  R --> S[install_hotfolder_service
  create script dir
  and hotfolder,
  install hotfolder_watcher.py,
  create hll1222-hotfolder.service,
  enable and start]
  S --> T[install_upload_service_optional
  if enabled, install
  upload_server.py and
  hll1222-upload.service,
  open firewall port,
  enable and start]
  T --> U[print_summary
  show IPP URI and
  optional upload URL]
  U --> Z[Exit]
Loading

File-Level Changes

Change Details Files
Introduce a full-stack setup script to provision the Raspberry Pi as a CUPS Android print server for the Brother HL-L1222-V.
  • Add a bash installer that installs required CUPS, Avahi, firewall, USB, Python, and optional Bluetooth/brlaser packages via zypper with non-interactive flags and error handling.
  • Implement CLI argument parsing for queue/device/model selection, Bluetooth and upload toggles, upload port, and a status-only diagnostic mode.
  • Add helpers for logging, cleanup, command existence checks, and explicit user confirmation for high‑risk operations (CUPS config edits, queue recreation).
  • Enable and start cups, avahi-daemon, firewalld, and optionally bluetooth via systemd, and open firewall services for IPP, mDNS, and an optional upload TCP port.
  • Detect the Brother USB device URI and select a print model preferring brlaser, then everywhere, then a generic mono laser, with fallbacks and diagnostics.
  • Create or replace the CUPS queue with selected device/model, set defaults (A4, one-sided), share it over the network, set it as default, and restart CUPS.
  • Configure CUPS sharing via cupsctl plus direct edits to cupsd.conf (WebInterface, Browsing, BrowseLocalProtocols dnssd) with timestamped backups.
scripts/setup_hl_l1222_android_print.sh
Add a Python-based hotfolder auto-print watcher and corresponding systemd unit running as lp.
  • Create a Python3 watcher that monitors a configured hotfolder, filters by allowed extensions, waits for file size stability, and submits jobs to the configured CUPS queue via lp.
  • Implement separate subdirectories for successfully printed and failed jobs, with timestamp-prefixed filenames and basic error isolation.
  • Install the watcher into a dedicated script directory, create the hotfolder owned by lp:lp, and define a systemd service that runs as lp with appropriate environment variables.
  • Enable and start the hotfolder watcher via systemd, ensuring dependency on cups and multi-user.target.
scripts/setup_hl_l1222_android_print.sh
Provide an optional LAN upload HTTP service that feeds into the hotfolder, plus systemd integration and firewall exposure.
  • Implement a minimal Python3 ThreadingHTTPServer-based upload endpoint that serves a simple HTML form, accepts multipart file uploads, enforces a maximum upload size, and restricts to specific file types.
  • Write uploads into the hotfolder with sanitized, de-duplicated filenames, suitable permissions, and environment-configurable hotfolder/queue/port/size limits.
  • Install the upload server script into the shared script directory and define a dedicated systemd service running as lp, dependent on network-online and the hotfolder service.
  • Open the configured upload TCP port in firewalld when upload is enabled, and enable/start the upload systemd unit conditionally based on CLI flags.
scripts/setup_hl_l1222_android_print.sh
Document the new setup workflow and Android IPP usage in the README.
  • Replace the placeholder README with a usage-oriented guide describing the Raspberry Pi + openSUSE Tumbleweed Android print server scenario and capabilities.
  • Document how to run the setup script with sudo, the expected queue name and IPP URI for Android/Mopria configuration, and the assumption of same-Wi-Fi connectivity.
  • Note the reliance on the CUPS everywhere model for broad compatibility and how to switch to an official Brother driver if available.
  • Clarify that Bluetooth is not a reliable Android printing path and that Wi‑Fi IPP is the primary supported workflow.
README.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • The --status flag handling is effectively broken because STATUS_ONLY is declared as a local in main() after parse_args mutates the global variable; the local shadowing means the status path is never taken—either remove the local or have parse_args return this via an explicit output mechanism.
  • When checking for existing CUPS directives in configure_cups_sharing, grep -qi will also match commented lines (e.g. # WebInterface No), so repeated runs may leave users with disabled-but-commented directives and no active ones; consider anchoring to non-comment lines (e.g. ^[[:space:]]*WebInterface[[:space:]]\+Yes) to ensure active configuration is enforced.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `--status` flag handling is effectively broken because `STATUS_ONLY` is declared as a local in `main()` after `parse_args` mutates the global variable; the local shadowing means the status path is never taken—either remove the `local` or have `parse_args` return this via an explicit output mechanism.
- When checking for existing CUPS directives in `configure_cups_sharing`, `grep -qi` will also match commented lines (e.g. `# WebInterface No`), so repeated runs may leave users with disabled-but-commented directives and no active ones; consider anchoring to non-comment lines (e.g. `^[[:space:]]*WebInterface[[:space:]]\+Yes`) to ensure active configuration is enforced.

## Individual Comments

### Comment 1
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="34" />
<code_context>
+HOTFOLDER_SERVICE="hll1222-hotfolder"
+UPLOAD_SERVICE="hll1222-upload"
+
+TMP_DIR=""
+CUPSD_CONF="/etc/cups/cupsd.conf"
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Resetting `TMP_DIR` to an empty string prevents the trap from cleaning up the temporary directory and leaks it.

Both `install_hotfolder_service` and `install_upload_service_optional` create a temp dir via `TMP_DIR="$(mktemp -d)"` and depend on the global `cleanup` trap to remove it. By resetting `TMP_DIR=""` at the end of these functions, the trap no longer knows which directory to delete and leaves the temp dir behind. Consider removing the `TMP_DIR=""` assignments so the trap can run correctly, or use function-local temp dirs that you explicitly `rm -rf` before clearing.
</issue_to_address>

### Comment 2
<location path="README.md" line_range="30" />
<code_context>
+
+## Notes
+- Script uses CUPS `everywhere` model as default for broad compatibility.
+- If you have an official Brother HL-L1222-V Linux PPD/driver package, install it and adjust queue model if desired.
</code_context>
<issue_to_address>
**nitpick (typo):** Consider adding an article before "queue model" for clarity.

This small tweak preserves the meaning but improves the sentence flow.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

HOTFOLDER_SERVICE="hll1222-hotfolder"
UPLOAD_SERVICE="hll1222-upload"

TMP_DIR=""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): Resetting TMP_DIR to an empty string prevents the trap from cleaning up the temporary directory and leaks it.

Both install_hotfolder_service and install_upload_service_optional create a temp dir via TMP_DIR="$(mktemp -d)" and depend on the global cleanup trap to remove it. By resetting TMP_DIR="" at the end of these functions, the trap no longer knows which directory to delete and leaves the temp dir behind. Consider removing the TMP_DIR="" assignments so the trap can run correctly, or use function-local temp dirs that you explicitly rm -rf before clearing.

Comment thread README.md

## Notes
- Script uses CUPS `everywhere` model as default for broad compatibility.
- If you have an official Brother HL-L1222-V Linux PPD/driver package, install it and adjust queue model if desired.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nitpick (typo): Consider adding an article before "queue model" for clarity.

This small tweak preserves the meaning but improves the sentence flow.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82f6567f7f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +298 to +300
MODEL_NAME="everywhere"
warn "Falling back to 'everywhere' model."
return 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid selecting everywhere for USB backend URIs

When brlaser is unavailable, this code falls back to MODEL_NAME="everywhere" and then uses it with whatever DEVICE_URI was auto-detected, which is typically usb://... for this workflow. lpadmin(8) documents that the everywhere model queries the printer via an IPP device URI, so using it with a USB backend can make queue creation fail on exactly the fallback path this script is meant to rescue. Please gate this fallback to IPP/driverless URIs (or prefer a generic PPD for USB) to avoid setup failures on common HL-L1222 USB deployments.

Useful? React with 👍 / 👎.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a comprehensive setup script for configuring a Raspberry Pi running openSUSE Tumbleweed as an Android-compatible print server for Brother HL-L1222 printers. The implementation includes automated CUPS configuration, firewall management, and Python-based services for hotfolder monitoring and web-based file uploads. Feedback focuses on improving the robustness of the Python scripts, specifically addressing the deprecation of the cgi module, fixing a resource leak in temporary directory cleanup, and using higher-precision timestamps to prevent file collisions. Additionally, suggestions were made to use cupsctl for safer configuration management and to refine IP address detection for multi-interface environments.


cat > "$upload_py" <<'PY'
#!/usr/bin/env python3
import cgi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The cgi module is deprecated in Python 3.11 and has been removed in Python 3.13 (PEP 594). Since openSUSE Tumbleweed is a rolling release, it is likely to use a version of Python where this module is either deprecated or unavailable. This will cause the optional upload service to fail. Consider using a modern alternative for parsing multipart form data, such as the multipart library or a lightweight framework, although this may introduce external dependencies.

Comment on lines +249 to +256
echo 'WebInterface Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi
if ! sudo grep -qi '^Browsing[[:space:]]\+Yes' "${CUPSD_CONF}"; then
echo 'Browsing Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi
if ! sudo grep -qi '^BrowseLocalProtocols[[:space:]]\+dnssd' "${CUPSD_CONF}"; then
echo 'BrowseLocalProtocols dnssd' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Manually appending directives to cupsd.conf can lead to duplicate or conflicting entries if the keys already exist with different values. Using cupsctl is the recommended and safer way to modify these settings as it handles the configuration file logic correctly and ensures the service is aware of the changes.

Suggested change
echo 'WebInterface Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi
if ! sudo grep -qi '^Browsing[[:space:]]\+Yes' "${CUPSD_CONF}"; then
echo 'Browsing Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi
if ! sudo grep -qi '^BrowseLocalProtocols[[:space:]]\+dnssd' "${CUPSD_CONF}"; then
echo 'BrowseLocalProtocols dnssd' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi
if ! sudo cupsctl WebInterface=Yes Browsing=Yes BrowseLocalProtocols=dnssd; then\n err "Failed to configure CUPS sharing settings via cupsctl."\n exit 1\n fi

exit 1
fi

TMP_DIR=""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Setting TMP_DIR="" without first removing the directory causes a resource leak. The temporary directory created by mktemp -d remains on disk, and because the variable is cleared, the cleanup trap will no longer be able to delete it on exit.

Suggested change
TMP_DIR=""
rm -rf "${TMP_DIR}"\n TMP_DIR=""

for f in sorted(hot.iterdir()):
if f.is_dir() or f.name.startswith('.'):
continue
tag = str(int(time.time())) + "-" + f.name

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using int(time.time()) as a filename prefix can lead to collisions if multiple files with the same name are processed within the same second. Using time.time_ns() provides much higher precision and reduces the risk of collisions.

Suggested change
tag = str(int(time.time())) + "-" + f.name
tag = f"{time.time_ns()}-{f.name}"


print_summary() {
local ip
ip="$(hostname -I | awk '{print $1}')"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

hostname -I | awk '{print $1}' returns the first IP address found, which may not be the correct one if the Raspberry Pi is connected to multiple networks (e.g., Ethernet and Wi-Fi). Since the primary goal is Android printing over Wi-Fi, it is safer to specifically identify the IP address on the wireless interface.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

3 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="README.md">

<violation number="1" location="README.md:29">
P3: README incorrectly states that `everywhere` is the default model; the script actually prefers `brlaser`/Brother models and uses `everywhere` only as fallback.</violation>
</file>

<file name="scripts/setup_hl_l1222_android_print.sh">

<violation number="1" location="scripts/setup_hl_l1222_android_print.sh:94">
P2: Options that require a value are not validated before reading `$2`, so missing values crash the script under `set -u` instead of returning a clear argument error.</violation>

<violation number="2" location="scripts/setup_hl_l1222_android_print.sh:122">
P2: Validate `--upload-port` as an integer in the valid TCP range before using it; invalid values currently break firewall/service setup.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

arg="$1"
case "$arg" in
--queue)
QUEUE_NAME="$2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Options that require a value are not validated before reading $2, so missing values crash the script under set -u instead of returning a clear argument error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 94:

<comment>Options that require a value are not validated before reading `$2`, so missing values crash the script under `set -u` instead of returning a clear argument error.</comment>

<file context>
@@ -0,0 +1,664 @@
+    arg="$1"
+    case "$arg" in
+      --queue)
+        QUEUE_NAME="$2"
+        shift 2
+        ;;
</file context>

shift
;;
--upload-port)
UPLOAD_PORT="$2"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Validate --upload-port as an integer in the valid TCP range before using it; invalid values currently break firewall/service setup.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/setup_hl_l1222_android_print.sh, line 122:

<comment>Validate `--upload-port` as an integer in the valid TCP range before using it; invalid values currently break firewall/service setup.</comment>

<file context>
@@ -0,0 +1,664 @@
+        shift
+        ;;
+      --upload-port)
+        UPLOAD_PORT="$2"
+        shift 2
+        ;;
</file context>

Comment thread README.md
- `ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V`

## Notes
- Script uses CUPS `everywhere` model as default for broad compatibility.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: README incorrectly states that everywhere is the default model; the script actually prefers brlaser/Brother models and uses everywhere only as fallback.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 29:

<comment>README incorrectly states that `everywhere` is the default model; the script actually prefers `brlaser`/Brother models and uses `everywhere` only as fallback.</comment>

<file context>
@@ -1,3 +1,30 @@
+   - `ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V`
+
+## Notes
+- Script uses CUPS `everywhere` model as default for broad compatibility.
+- If you have an official Brother HL-L1222-V Linux PPD/driver package, install it and adjust queue model if desired.
</file context>
Suggested change
- Script uses CUPS `everywhere` model as default for broad compatibility.
- Script prefers a `brlaser`/Brother model when available, then falls back to CUPS `everywhere` (and finally a generic monochrome laser model).

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 8 potential issues.

Open in Devin Review


cat > "$upload_py" <<'PY'
#!/usr/bin/env python3
import cgi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Upload server uses cgi module removed from Python 3.13+ stdlib

The embedded upload server script (upload_server.py) uses import cgi (line 486) and cgi.FieldStorage (line 525). The cgi module was deprecated in Python 3.11 (PEP 594) and removed from the standard library in Python 3.13 (released October 2024). Since this script targets openSUSE Tumbleweed (a rolling-release distro, see line 7) and installs the system python3 package (line 172), the installed Python version in April 2026 will be 3.13+. The upload service will crash on startup with ModuleNotFoundError: No module named 'cgi', making the --enable-upload feature completely non-functional.

Affected code using removed API

Line 486: import cgi
Line 525: form = cgi.FieldStorage(fp=self.rfile, headers=self.headers, ...)

Replace with email.parser / manual multipart parsing, or use the multipart PyPI package.

Prompt for agents
The upload server embedded in setup_hl_l1222_android_print.sh uses `import cgi` (line 486) and `cgi.FieldStorage` (line 525) for multipart form parsing. The `cgi` module was removed from Python's standard library in Python 3.13 (PEP 594). Since the script targets openSUSE Tumbleweed (rolling release) which ships Python 3.13+, the upload service will fail to start.

To fix this, replace the `cgi.FieldStorage` usage with one of:
1. The `multipart` PyPI package (would need `pip install multipart` in the setup script)
2. Manual multipart boundary parsing using the `email` stdlib module
3. A minimal inline multipart parser that reads from `self.rfile` using the boundary from the Content-Type header

The affected code is the `do_POST` method in class `H` (around line 517-548 in the heredoc). The `cgi.FieldStorage` call at line 525 parses the multipart form data to extract the uploaded file. The replacement needs to extract the file content and filename from the multipart POST body.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +537 to +541
i = 1
while dst.exists():
dst = hot / f"{Path(safe).stem}_{i}{ext}"
i += 1
with open(dst, "wb") as f:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 TOCTOU race in upload filename deduplication with ThreadingHTTPServer

The upload server uses ThreadingHTTPServer (line 553) for concurrent request handling, but the filename deduplication logic at lines 537–541 uses a non-atomic check-then-write pattern: dst.exists() is checked in a loop, then open(dst, "wb") writes to the chosen path. Two concurrent uploads with the same filename can both pass the existence check before either creates the file, causing one upload to silently overwrite the other and corrupting the file written by the first thread.

Prompt for agents
The upload server uses ThreadingHTTPServer (line 553) but the filename deduplication at lines 537-541 has a TOCTOU race condition. The `while dst.exists()` check and subsequent `open(dst, 'wb')` are not atomic, so concurrent uploads with the same filename can collide.

Possible fixes:
1. Use `os.open(dst, os.O_CREAT | os.O_EXCL | os.O_WRONLY)` to atomically create the file, retrying with incremented suffix on `FileExistsError`.
2. Add a threading.Lock around the dedup-and-write section.
3. Include a unique component (e.g., UUID or thread ID) in the filename to avoid collisions entirely.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

exit 1
fi

TMP_DIR=""

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Temp directories leaked on successful script completion

Both install_hotfolder_service (line 470) and install_upload_service_optional (line 606) set TMP_DIR="" after completing their work. This orphans the actual temp directories created by mktemp -d at lines 372 and 480. The cleanup trap at scripts/setup_hl_l1222_android_print.sh:41-44 only removes the directory referenced by the current value of TMP_DIR, which is empty on normal exit. Additionally, the first temp dir (line 372) is always leaked even on errors during the second function, because TMP_DIR was already cleared at line 470.

Prompt for agents
The cleanup function at lines 41-44 only tracks a single TMP_DIR value, but install_hotfolder_service and install_upload_service_optional each create a separate temp directory and then set TMP_DIR to empty after use. This means neither temp directory is ever cleaned up by the trap.

Possible fixes:
1. Track temp directories in an array (e.g., TMP_DIRS) and iterate over all entries in the cleanup function.
2. Remove each temp directory immediately after its contents have been installed (i.e., replace `TMP_DIR=""` with `rm -rf "$TMP_DIR"; TMP_DIR=""`).
3. Use a single shared temp directory created once at the start of main(), with cleanup at exit.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +248 to +256
if ! sudo grep -qi '^WebInterface[[:space:]]\+Yes' "${CUPSD_CONF}"; then
echo 'WebInterface Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi
if ! sudo grep -qi '^Browsing[[:space:]]\+Yes' "${CUPSD_CONF}"; then
echo 'Browsing Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi
if ! sudo grep -qi '^BrowseLocalProtocols[[:space:]]\+dnssd' "${CUPSD_CONF}"; then
echo 'BrowseLocalProtocols dnssd' | sudo tee -a "${CUPSD_CONF}" >/dev/null
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚩 CUPS config grep-and-append may create duplicate directives

Lines 248-256 check for specific directive+value patterns (e.g., Browsing Yes) and append if not found. However, these greps won't match semantically equivalent values like Browsing On (which cupsctl --share-printers at line 242 may have already set). This would result in duplicate directives in cupsd.conf. Whether this causes a problem depends on CUPS' handling of duplicates — some CUPS versions use the first occurrence, others use the last. A safer approach would be to use sed to replace or remove existing conflicting lines before appending, rather than relying on grep for a specific value.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +640 to +649
local STATUS_ONLY="no"
parse_args "$@"

require_command "sudo" "sudo"
require_command "zypper" "zypper"
require_command "lpinfo" "cups-client"
require_command "lpadmin" "cups-client"
require_command "lpstat" "cups-client"

if [[ "${STATUS_ONLY:-no}" == "yes" ]]; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: STATUS_ONLY scoping between parse_args and main works correctly in bash

At first glance, STATUS_ONLY appears problematic: it's declared local in main() (line 640) but assigned in parse_args() (line 126) without local. In bash, called functions can see and modify the calling function's local variables, so parse_args correctly modifies main's local STATUS_ONLY. The ${STATUS_ONLY:-no} fallback at line 649 is also redundant since the local is always initialized to "no", but it's harmless defensive coding.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +535 to +536
safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160]
dst = hot / safe

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Uploaded files with dot-prefixed names silently skip printing

The upload server's filename sanitization (line 535) preserves . characters, so an uploaded file named .config.pdf would be saved as .config.pdf in the hotfolder. The hotfolder watcher at line 406 skips files starting with . (f.name.startswith('.')). This means such files are accepted and saved but never printed and never moved to failed/ — they accumulate silently in the hotfolder. The impact is very low (browser uploads rarely produce dot-prefixed names) but a safe = safe.lstrip('.') after sanitization would close the gap.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread README.md

## Run
```bash
sudo bash scripts/setup_hl_l1222_android_print.sh

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚩 README says sudo bash but script comments say run as normal user

The README at line 18 instructs users to run sudo bash scripts/setup_hl_l1222_android_print.sh, which executes the entire script as root. However, the script's own header comment (scripts/setup_hl_l1222_android_print.sh:11-13) says "Run as a normal user from home directory (~/)" and uses sudo internally for privileged operations. Running as root makes all internal sudo calls redundant and could subtly change behavior (e.g., lpoptions -d at line 349 sets the default printer for root rather than a regular user). This inconsistency should be resolved.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

if ext not in allowed:
self.send_body(400, f"Unsupported file type: {html.escape(ext)}")
return
safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Filename truncation to 160 chars can break file extension

At line 535, the sanitized filename is truncated to 160 characters: safe = ...[:160]. If the original filename is long enough, this truncation can cut through the file extension (e.g., a 200-char name ending in .pdf becomes truncated to 160 chars ending in .pd). The hotfolder watcher checks f.suffix.lower() not in allowed (line 409), so a file with a broken extension would be moved to the failed/ folder rather than printed. The upload server would report success (line 548) even though the file will never print. This is an edge case with very long filenames but leads to silent data loss.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant