Skip to content

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

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

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

Conversation

@ib-bsb-br

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

Copy link
Copy Markdown
Owner

Motivation

  • Provide an automated, repeatable setup to configure a Raspberry Pi 4B+ running openSUSE Tumbleweed as a CUPS-backed print server for a USB-connected Brother HL-L1222/HL-L1222-V.
  • Enable Android devices on the same Wi-Fi to print via IPP/Mopria and configure discovery and firewall for LAN printing.
  • Document the recommended Wi-Fi IPP workflow and caveat Bluetooth printing limitations for modern Android/CUPS stacks.

Description

  • Add an executable setup script scripts/setup_hl_l1222_android_print.sh that installs required packages with zypper, enables services via systemctl, and configures CUPS, Avahi, Firewalld, and Bluetooth.
  • Script edits /etc/cups/cupsd.conf to enable network listening, sharing, and web interface, opens firewall rules for ipp and mdns via firewall-cmd, and attempts to auto-detect the Brother USB URI using lpinfo.
  • The script creates or replaces the CUPS queue with lpadmin using the everywhere model, sets default options (one-sided, A4), enables sharing, restarts CUPS, and issues a test print and lpstat -t status.
  • Update README.md with usage instructions, the recommended IPP ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V workflow, and notes about Bluetooth and driver options.

Testing

  • No automated tests were run for this change.

Codex Task


Open in Devin Review

Summary by Sourcery

Add a setup script and documentation to configure a Raspberry Pi (openSUSE Tumbleweed) as an IPP/Mopria-compatible print server for a USB-connected Brother HL-L1222/HL-L1222-V printer, enabling Android Wi‑Fi printing.

New Features:

  • Introduce an executable Bash script to automatically install and configure CUPS, discovery services, firewall rules, and a shared print queue for a Brother HL-L1222/HL-L1222-V on openSUSE Tumbleweed.
  • Add user-facing documentation describing how to run the setup script and connect Android devices to the configured IPP printer over Wi‑Fi.

Enhancements:

  • Configure CUPS to listen on the local network, enable browsing and sharing, and expose the web interface for administration.
  • Open and persist firewall rules for IPP and mDNS to allow printer discovery and LAN printing.
  • Enable and start auxiliary services (Avahi, firewalld, Bluetooth) needed for discovery and optional connectivity, and set sensible default print options such as A4, one-sided printing.

Documentation:

  • Expand README with setup instructions, recommended Android IPP/Mopria workflow, printer URI format, and notes on Bluetooth and driver options.

Summary by cubic

Adds an automated setup script to turn a Raspberry Pi 4B+ (openSUSE Tumbleweed) into a CUPS IPP print server for a USB Brother HL‑L1222/HL‑L1222‑V. Updates the README with Android IPP steps and Bluetooth caveats.

  • New Features
    • Added scripts/setup_hl_l1222_android_print.sh to install deps (cups, cups-filters, ghostscript, avahi, nss-mdns, firewalld, ipp-usb, bluez, bluez-tools) via zypper, enable services via systemctl, and configure CUPS, Avahi, and the firewall.
    • Configures /etc/cups/cupsd.conf (Port 631, browsing, sharing, web UI, allow @LOCAL) and opens ipp/mdns in firewalld.
    • Auto-detects the Brother USB URI, creates a shared Brother_HL_L1222_V queue with model everywhere, sets A4 and one-sided, restarts CUPS, prints a test page, and shows status.
    • Updated README.md with run instructions and the Android IPP URL: ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V, plus a note that Bluetooth printing is unreliable; use Wi‑Fi IPP/Mopria.

Written for commit df4d1d3. 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 a bash setup script to turn a Raspberry Pi 4B+ running openSUSE Tumbleweed into an IPP/Mopria-capable CUPS print server for a USB-connected Brother HL-L1222(-V), and updates the README with usage and Android printing instructions.

Sequence diagram for Android IPP printing via Raspberry Pi CUPS server

sequenceDiagram
    actor AndroidUser
    participant AndroidDevice
    participant WiFiRouter
    participant RaspberryPi_CUPS
    participant USB_Printer

    AndroidUser->>AndroidDevice: Select print and confirm job
    AndroidDevice->>WiFiRouter: Send IPP print job
    WiFiRouter->>RaspberryPi_CUPS: Forward IPP traffic to port 631
    RaspberryPi_CUPS->>RaspberryPi_CUPS: CUPS processes IPP job
    RaspberryPi_CUPS->>USB_Printer: Send rasterized job via USB
    USB_Printer-->>RaspberryPi_CUPS: Print status
    RaspberryPi_CUPS-->>AndroidDevice: IPP job status response
    AndroidDevice-->>AndroidUser: Show job completed/failed status
Loading

Flow diagram for setup_hl_l1222_android_print.sh script logic

flowchart TD
    S[Start script
check root] --> CRoot{EUID is 0?}
    CRoot -- No --> E1[Print run as root message
Exit 1]
    CRoot -- Yes --> CheckCmds[Verify required commands
zypper systemctl lpinfo lpadmin lpstat]

    CheckCmds --> Install[Install packages via zypper
cups cups-filters ghostscript
avahi nss-mdns firewalld
ipp-usb bluez bluez-tools]

    Install --> EnableSvc[Enable and start services
cups avahi-daemon
firewalld bluetooth]

    EnableSvc --> EditCUPS[Edit /etc/cups/cupsd.conf
Port 631
Browsing Yes
DefaultShared Yes
WebInterface Yes
<Location /> allow @LOCAL]

    EditCUPS --> Firewall[Open firewall services
ipp mdns and reload]

    Firewall --> DetectURI[Detect Brother USB URI via lpinfo]

    DetectURI --> HasURI{Found Brother USB URI?}
    HasURI -- No --> ShowDevices[Print available devices
via lpinfo]
    ShowDevices --> E2[Prompt to connect printer
and rerun script
Exit 1]
    HasURI -- Yes --> QueueCheck{Existing queue
Brother_HL_L1222_V?}

    QueueCheck -- Yes --> DeleteQueue[Delete existing queue
lpadmin -x]
    QueueCheck -- No --> CreateQueue

    DeleteQueue --> CreateQueue[Create queue with lpadmin
model everywhere]

    CreateQueue --> SetOptions[Set default options
sides=one-sided media=A4
printer-is-shared=true]

    SetOptions --> RestartCUPS[Restart cups.service]

    RestartCUPS --> TestPrint[Print test page
lp testprint]

    TestPrint --> Status[Show status
lpstat -t]

    Status --> Done[Print Android IPP instructions
and Bluetooth note
End]
Loading

File-Level Changes

Change Details Files
Introduce a bash setup script that installs and configures the printing/network stack and CUPS queue for the Brother HL-L1222(-V) on openSUSE Tumbleweed.
  • Add root-only bash script with strict error handling and required command checks
  • Install CUPS, filters, Avahi, firewalld, ipp-usb, and Bluetooth components via zypper
  • Enable and start cups, avahi-daemon, firewalld, and bluetooth services with systemctl
scripts/setup_hl_l1222_android_print.sh
Configure CUPS, firewall, and service discovery for LAN IPP/Mopria printing from Android devices.
  • Adjust cupsd.conf to listen on port 631, enable browsing/sharing/web interface, and allow @Local access for root and admin locations
  • Open ipp and mdns services in firewalld permanently and reload rules
  • Detect Brother USB printer URI with lpinfo and (re)create a shared CUPS queue using the everywhere model with sane defaults, then restart CUPS, print a test page, and show lpstat status
scripts/setup_hl_l1222_android_print.sh
Document the new setup script and the recommended Android IPP workflow in the README.
  • Replace placeholder README with project description and capabilities summary
  • Document how to run the setup script and the IPP URI to use from Android (Mopria)
  • Add notes about Bluetooth limitations and optional use of vendor PPD/driver instead of the everywhere model
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:

  • Before mutating /etc/cups/cupsd.conf, consider creating a timestamped backup so reruns or manual recovery are easier if the edits or package versions change in the future.
  • The USB URI detection is very specific to usb://Brother/ and fails hard if not found; consider allowing an override via a script argument or prompting the user to select from lpinfo -v instead of exiting.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Before mutating `/etc/cups/cupsd.conf`, consider creating a timestamped backup so reruns or manual recovery are easier if the edits or package versions change in the future.
- The USB URI detection is very specific to `usb://Brother/` and fails hard if not found; consider allowing an override via a script argument or prompting the user to select from `lpinfo -v` instead of exiting.

## Individual Comments

### Comment 1
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="67-72" />
<code_context>
+CUPSLOC
+fi
+
+echo "[3/8] Opening firewall for IPP + mDNS..."
+firewall-cmd --permanent --add-service=ipp || true
+firewall-cmd --permanent --add-service=mdns || true
+firewall-cmd --reload || true
+
+echo "[4/8] Detecting USB printer URI..."
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid fully swallowing firewall-cmd failures or at least surface them to the user.

Using `|| true` on all three `firewall-cmd` calls means the script always reports success even if firewalld is missing or the rules fail to apply, which can leave network printing broken without any indication. Consider only relaxing failures when firewalld is intentionally inactive (e.g., via `systemctl is-active firewalld`) and/or logging a clear warning when `firewall-cmd` fails so users know IPP/mDNS might not be open.

```suggestion
echo "[3/8] Opening firewall for IPP + mDNS..."
if ! command -v firewall-cmd >/dev/null 2>&1; then
  echo "Warning: firewall-cmd not found; skipping firewall configuration. Network printing may not be reachable from other devices."
elif ! systemctl is-active --quiet firewalld; then
  echo "Warning: firewalld service is not active; skipping firewall configuration. Network printing may not be reachable from other devices."
else
  if ! firewall-cmd --permanent --add-service=ipp; then
    echo "Warning: failed to add IPP service to firewalld. Network printing may not be reachable from other devices."
  fi
  if ! firewall-cmd --permanent --add-service=mdns; then
    echo "Warning: failed to add mDNS service to firewalld. Network printer discovery may not work."
  fi
  if ! firewall-cmd --reload; then
    echo "Warning: failed to reload firewalld. Newly added firewall rules may not be active."
  fi
fi

echo "[4/8] Detecting USB printer URI..."
```
</issue_to_address>

### Comment 2
<location path="README.md" line_range="29" />
<code_context>
+   - `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.
</code_context>
<issue_to_address>
**nitpick (typo):** Consider adding an article: "as the default" for smoother grammar.

Suggestion: "Script uses CUPS `everywhere` model as the default for broad compatibility."

```suggestion
- Script uses CUPS `everywhere` model as the default for broad compatibility.
```
</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.

Comment on lines +67 to +72
echo "[3/8] Opening firewall for IPP + mDNS..."
firewall-cmd --permanent --add-service=ipp || true
firewall-cmd --permanent --add-service=mdns || true
firewall-cmd --reload || true

echo "[4/8] Detecting USB printer URI..."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

suggestion (bug_risk): Avoid fully swallowing firewall-cmd failures or at least surface them to the user.

Using || true on all three firewall-cmd calls means the script always reports success even if firewalld is missing or the rules fail to apply, which can leave network printing broken without any indication. Consider only relaxing failures when firewalld is intentionally inactive (e.g., via systemctl is-active firewalld) and/or logging a clear warning when firewall-cmd fails so users know IPP/mDNS might not be open.

Suggested change
echo "[3/8] Opening firewall for IPP + mDNS..."
firewall-cmd --permanent --add-service=ipp || true
firewall-cmd --permanent --add-service=mdns || true
firewall-cmd --reload || true
echo "[4/8] Detecting USB printer URI..."
echo "[3/8] Opening firewall for IPP + mDNS..."
if ! command -v firewall-cmd >/dev/null 2>&1; then
echo "Warning: firewall-cmd not found; skipping firewall configuration. Network printing may not be reachable from other devices."
elif ! systemctl is-active --quiet firewalld; then
echo "Warning: firewalld service is not active; skipping firewall configuration. Network printing may not be reachable from other devices."
else
if ! firewall-cmd --permanent --add-service=ipp; then
echo "Warning: failed to add IPP service to firewalld. Network printing may not be reachable from other devices."
fi
if ! firewall-cmd --permanent --add-service=mdns; then
echo "Warning: failed to add mDNS service to firewalld. Network printer discovery may not work."
fi
if ! firewall-cmd --reload; then
echo "Warning: failed to reload firewalld. Newly added firewall rules may not be active."
fi
fi
echo "[4/8] Detecting USB printer URI..."

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.

nitpick (typo): Consider adding an article: "as the default" for smoother grammar.

Suggestion: "Script uses CUPS everywhere model as the default for broad compatibility."

Suggested change
- Script uses CUPS `everywhere` model as default for broad compatibility.
- Script uses CUPS `everywhere` model as the default for broad compatibility.

@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 setup script and documentation to configure a Raspberry Pi running openSUSE Tumbleweed as a print server for the Brother HL-L1222-V printer, enabling Android printing via IPP/Mopria. The review identifies critical issues with the manual configuration of /etc/cups/cupsd.conf, suggesting the use of cupsctl for more reliable and idempotent setup. Additionally, it points out a compatibility conflict between the everywhere PPD model and the usb:// URI for this specific printer, recommending the use of ipp-usb discovery or the brlaser driver to ensure a functional print queue.

Comment on lines +34 to +65
# Ensure cupsd listens on local network for Android clients using IPP/Mopria.
CUPSD_CONF="/etc/cups/cupsd.conf"
if ! grep -q '^Port 631' "$CUPSD_CONF"; then
sed -i 's/^Listen localhost:631/# &/' "$CUPSD_CONF" || true
echo 'Port 631' >> "$CUPSD_CONF"
fi

if ! grep -q '^Browsing Yes' "$CUPSD_CONF"; then
echo 'Browsing Yes' >> "$CUPSD_CONF"
fi

if ! grep -q '^DefaultShared Yes' "$CUPSD_CONF"; then
echo 'DefaultShared Yes' >> "$CUPSD_CONF"
fi

if ! grep -q '^WebInterface Yes' "$CUPSD_CONF"; then
echo 'WebInterface Yes' >> "$CUPSD_CONF"
fi

if ! grep -q '<Location />' "$CUPSD_CONF"; then
cat >> "$CUPSD_CONF" <<'CUPSLOC'
<Location />
Order allow,deny
Allow @LOCAL
</Location>

<Location /admin>
Order allow,deny
Allow @LOCAL
</Location>
CUPSLOC
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.

high

The manual modification of /etc/cups/cupsd.conf is fragile and contains logic errors:

  1. Logic Error: The check grep -q '<Location />' will return true on most default CUPS installations because the block usually exists (though restricted). Consequently, the script will skip adding the Allow @LOCAL directive, and network printing will remain blocked ('Forbidden').
  2. Port Conflict: The sed command only targets localhost:631. If the configuration uses 127.0.0.1:631 or *:631, it won't be commented out, and appending Port 631 will cause CUPS to fail to start due to a port binding conflict.

Using cupsctl is the standard, safe, and idempotent way to configure these settings.

Suggested change
# Ensure cupsd listens on local network for Android clients using IPP/Mopria.
CUPSD_CONF="/etc/cups/cupsd.conf"
if ! grep -q '^Port 631' "$CUPSD_CONF"; then
sed -i 's/^Listen localhost:631/# &/' "$CUPSD_CONF" || true
echo 'Port 631' >> "$CUPSD_CONF"
fi
if ! grep -q '^Browsing Yes' "$CUPSD_CONF"; then
echo 'Browsing Yes' >> "$CUPSD_CONF"
fi
if ! grep -q '^DefaultShared Yes' "$CUPSD_CONF"; then
echo 'DefaultShared Yes' >> "$CUPSD_CONF"
fi
if ! grep -q '^WebInterface Yes' "$CUPSD_CONF"; then
echo 'WebInterface Yes' >> "$CUPSD_CONF"
fi
if ! grep -q '<Location />' "$CUPSD_CONF"; then
cat >> "$CUPSD_CONF" <<'CUPSLOC'
<Location />
Order allow,deny
Allow @LOCAL
</Location>
<Location /admin>
Order allow,deny
Allow @LOCAL
</Location>
CUPSLOC
fi
# Configure CUPS for network access and sharing
cupsctl --remote-any --share-printers --remote-admin WebInterface=Yes

if lpstat -p "$PRINTER_NAME" >/dev/null 2>&1; then
lpadmin -x "$PRINTER_NAME"
fi
lpadmin -p "$PRINTER_NAME" -E -v "$USB_URI" -m "$PPD_MODEL"

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 everywhere model (IPP Everywhere) is incompatible with a raw usb:// URI for the Brother HL-L1222-V, as this printer does not support IPP natively over USB. Using this combination will result in a non-functional print queue.

Since ipp-usb is installed (line 26), it may claim the device and provide an ipp:// or driverless:// URI. If you intend to use the everywhere model, you should use one of those URIs discovered via lpinfo. Otherwise, for this specific model, you likely need the brlaser driver (e.g., -m drv:///brlaser.drv/brl1222.ppd).

exit 1
fi

REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)

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

Add cupsctl to the list of required commands to ensure it is available for the configuration steps.

Suggested change
REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)
REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat cupsctl)

@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: df4d1d348f

ℹ️ 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 +17 to +19
REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)
for c in "${REQ_CMDS[@]}"; do
command -v "$c" >/dev/null || { echo "Missing command: $c"; exit 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.

P1 Badge Remove CUPS CLI prereq checks before package install

The script checks for lpinfo, lpadmin, and lpstat before running zypper install, so on a fresh openSUSE host (the primary bootstrap case) it exits with “Missing command” and never reaches the package installation step that would provide those binaries. This makes the setup non-functional unless CUPS client tools are already preinstalled.

Useful? React with 👍 / 👎.

if lpstat -p "$PRINTER_NAME" >/dev/null 2>&1; then
lpadmin -x "$PRINTER_NAME"
fi
lpadmin -p "$PRINTER_NAME" -E -v "$USB_URI" -m "$PPD_MODEL"

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 Use IPP URI when configuring the everywhere model

The queue is created with -m everywhere, but USB_URI is constrained to direct usb://Brother/...; per lpadmin(8), everywhere queries the printer via a specified IPP device URI. With a direct USB URI this frequently fails queue creation (or produces a non-working driverless setup), breaking the script’s default path for this printer.

Useful? React with 👍 / 👎.

@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 6 potential issues.

Open in Devin Review

Comment on lines +17 to +20
REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)
for c in "${REQ_CMDS[@]}"; do
command -v "$c" >/dev/null || { echo "Missing command: $c"; exit 1; }
done

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 commands (lpinfo, lpadmin, lpstat) are checked before CUPS is installed, blocking fresh installs

Lines 17-19 check that lpinfo, lpadmin, and lpstat are available via command -v, but these are CUPS binaries provided by the cups package which isn't installed until lines 24-26. On a fresh system (the primary target for a setup script), the prerequisite check fails with "Missing command: lpinfo" and exits before the zypper install cups step ever runs. The check should either move these CUPS-specific commands after the install step, or only verify base OS commands (zypper, systemctl) before installation.

Suggested change
REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)
for c in "${REQ_CMDS[@]}"; do
command -v "$c" >/dev/null || { echo "Missing command: $c"; exit 1; }
done
REQ_CMDS=(zypper systemctl)
for c in "${REQ_CMDS[@]}"; do
command -v "$c" >/dev/null || { echo "Missing command: $c"; exit 1; }
done
Open in Devin Review

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

Comment on lines +53 to +65
if ! grep -q '<Location />' "$CUPSD_CONF"; then
cat >> "$CUPSD_CONF" <<'CUPSLOC'
<Location />
Order allow,deny
Allow @LOCAL
</Location>

<Location /admin>
Order allow,deny
Allow @LOCAL
</Location>
CUPSLOC
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.

🔴 guard skips adding Allow @Local when default CUPS block already exists

Line 53 checks whether <Location /> already exists in cupsd.conf. The default CUPS installation ships with a <Location /> block that typically does not include Allow @LOCAL. Because the grep finds the existing block, the script skips appending the new block with Allow @LOCAL, so LAN clients (i.e. Android phones) are still denied access. This silently defeats the script's primary purpose — enabling network printing over IPP/Mopria. The fix should either inject Allow @LOCAL into the existing <Location /> block, or replace the existing block rather than simply checking for its presence.

Prompt for agents
The guard on line 53 checks for the presence of `<Location />` in cupsd.conf and skips the entire block if found. However, the default CUPS cupsd.conf already contains a `<Location />` directive — typically without `Allow @LOCAL`. This means the script never adds the crucial `Allow @LOCAL` rule that enables LAN printing.

The fix should handle two scenarios:
1. If `<Location />` already exists but lacks `Allow @LOCAL`, inject `Allow @LOCAL` into the existing block (e.g. using sed to insert after the `Order allow,deny` line within `<Location />`). Do the same for `<Location /admin>`.
2. If `<Location />` doesn't exist at all, append the full block as currently done.

Also consider checking for `Allow @LOCAL` specifically (e.g., `grep -q 'Allow @LOCAL' cupsd.conf`) rather than checking for `<Location />`.

Relevant function: the block at scripts/setup_hl_l1222_android_print.sh lines 53-65.
Open in Devin Review

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

firewall-cmd --reload || true

echo "[4/8] Detecting USB printer URI..."
USB_URI="$(lpinfo -v | awk '/^direct usb:\/\/Brother\//{print $2; exit}')"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚩 ipp-usb package may conflict with direct USB backend for printer detection

The script installs ipp-usb at scripts/setup_hl_l1222_android_print.sh:26, which provides an IPP-over-USB bridge. Once running, ipp-usb may take exclusive ownership of the USB device, causing the lpinfo -v detection at line 73 to fail to find the direct usb://Brother/... URI. The printer might instead appear only as an ipp://localhost:60000/... endpoint. If this happens, the awk filter /^direct usb:\/\/Brother\// wouldn't match and the script would exit at line 78 claiming no printer was found. Whether this actually occurs depends on the ipp-usb daemon activation timing and USB device locking behavior on the target platform — it warrants testing.

Open in Devin Review

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

Comment on lines +97 to +100
systemctl restart cups.service

echo "[7/8] Printing a test page..."
lp -d "$PRINTER_NAME" /usr/share/cups/data/testprint || true

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: Test print may silently fail due to CUPS restart timing

At line 97, CUPS is restarted with systemctl restart cups.service, and the very next command at line 100 submits a test print job via lp. There is no delay or readiness check between the restart and the print submission. If CUPS hasn't fully started accepting connections, lp would fail. The || true prevents the script from aborting, but the test page would silently not print. This is non-critical (all configuration is already persisted), but could confuse users who expect to see a test page.

Open in Devin Review

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

lp -d "$PRINTER_NAME" /usr/share/cups/data/testprint || true

echo "[8/8] Final status"
lpstat -t

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: lpstat -t at end of script can abort under set -e if CUPS is still restarting

Line 103 runs lpstat -t to display final status. Unlike the test print on line 100 which has || true, this command has no error suppression. Under set -e, if CUPS hasn't fully restarted yet (same timing concern as ANALYSIS-0002), lpstat -t would fail and cause the script to exit with an error after all setup is already complete. This would give a misleading impression of failure. Adding || true or a brief sleep before status commands would make the script more robust.

Open in Devin Review

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

Comment on lines +36 to +51
if ! grep -q '^Port 631' "$CUPSD_CONF"; then
sed -i 's/^Listen localhost:631/# &/' "$CUPSD_CONF" || true
echo 'Port 631' >> "$CUPSD_CONF"
fi

if ! grep -q '^Browsing Yes' "$CUPSD_CONF"; then
echo 'Browsing Yes' >> "$CUPSD_CONF"
fi

if ! grep -q '^DefaultShared Yes' "$CUPSD_CONF"; then
echo 'DefaultShared Yes' >> "$CUPSD_CONF"
fi

if ! grep -q '^WebInterface Yes' "$CUPSD_CONF"; then
echo 'WebInterface Yes' >> "$CUPSD_CONF"
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.

📝 Info: Appending directives to cupsd.conf creates ordering issues

Lines 36-51 append Port 631, Browsing Yes, DefaultShared Yes, and WebInterface Yes to the end of cupsd.conf. CUPS processes directives in order, and some of these may conflict with or be overridden by earlier directives already in the file. For example, if the original file has Browsing No near the top, appending Browsing Yes at the bottom means CUPS sees both — and the last value typically wins. While this happens to work in CUPS's favor for this script, it leaves behind contradictory entries. A cleaner approach would be sed -i to replace existing directives in-place rather than appending duplicates.

Open in Devin Review

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

@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.

4 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="scripts/setup_hl_l1222_android_print.sh">

<violation number="1" location="scripts/setup_hl_l1222_android_print.sh:17">
P1: Preflight checks require CUPS commands before CUPS is installed, so the script can fail immediately on fresh systems.</violation>

<violation number="2" location="scripts/setup_hl_l1222_android_print.sh:53">
P1: Logic error: `grep -q '<Location />'` will match the existing `<Location />` block present in default CUPS configurations (which typically has restrictive ACLs). Since the block already exists, this check passes and the script never adds `Allow @LOCAL`, leaving network access blocked (HTTP 403 Forbidden for LAN clients).

Additionally, the `sed` on line 37 only targets `Listen localhost:631` — if the config uses `127.0.0.1:631` or `*:631`, it won't be commented out, and appending `Port 631` will cause a port binding conflict that prevents CUPS from starting.

Consider using `cupsctl --remote-any --share-printers` which handles these cases safely and idempotently.</violation>

<violation number="3" location="scripts/setup_hl_l1222_android_print.sh:68">
P2: Do not swallow firewall command failures; this can produce a false successful setup with broken network printing.</violation>

<violation number="4" location="scripts/setup_hl_l1222_android_print.sh:87">
P0: The `everywhere` model requires an IPP device URI to query the printer's capabilities (per `lpadmin(8)`: "the model 'everywhere' queries the printer referred to by the specified IPP device-uri"). The `USB_URI` here is a `usb://` scheme which cannot be queried via IPP, so this will either fail queue creation or produce a non-functional driverless queue.

Since `ipp-usb` is installed, you should detect the `ipp://` or `driverless://` URI it provides (via `lpinfo -v`), or fall back to a compatible driver like `brlaser` (e.g., `-m drv:///brlaser.drv/brl1222.ppd`) for the USB path.</violation>
</file>

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

if lpstat -p "$PRINTER_NAME" >/dev/null 2>&1; then
lpadmin -x "$PRINTER_NAME"
fi
lpadmin -p "$PRINTER_NAME" -E -v "$USB_URI" -m "$PPD_MODEL"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P0: The everywhere model requires an IPP device URI to query the printer's capabilities (per lpadmin(8): "the model 'everywhere' queries the printer referred to by the specified IPP device-uri"). The USB_URI here is a usb:// scheme which cannot be queried via IPP, so this will either fail queue creation or produce a non-functional driverless queue.

Since ipp-usb is installed, you should detect the ipp:// or driverless:// URI it provides (via lpinfo -v), or fall back to a compatible driver like brlaser (e.g., -m drv:///brlaser.drv/brl1222.ppd) for the USB path.

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 87:

<comment>The `everywhere` model requires an IPP device URI to query the printer's capabilities (per `lpadmin(8)`: "the model 'everywhere' queries the printer referred to by the specified IPP device-uri"). The `USB_URI` here is a `usb://` scheme which cannot be queried via IPP, so this will either fail queue creation or produce a non-functional driverless queue.

Since `ipp-usb` is installed, you should detect the `ipp://` or `driverless://` URI it provides (via `lpinfo -v`), or fall back to a compatible driver like `brlaser` (e.g., `-m drv:///brlaser.drv/brl1222.ppd`) for the USB path.</comment>

<file context>
@@ -0,0 +1,118 @@
+if lpstat -p "$PRINTER_NAME" >/dev/null 2>&1; then
+  lpadmin -x "$PRINTER_NAME"
+fi
+lpadmin -p "$PRINTER_NAME" -E -v "$USB_URI" -m "$PPD_MODEL"
+lpoptions -d "$PRINTER_NAME"
+
</file context>

exit 1
fi

REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)

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: Preflight checks require CUPS commands before CUPS is installed, so the script can fail immediately on fresh systems.

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 17:

<comment>Preflight checks require CUPS commands before CUPS is installed, so the script can fail immediately on fresh systems.</comment>

<file context>
@@ -0,0 +1,118 @@
+  exit 1
+fi
+
+REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)
+for c in "${REQ_CMDS[@]}"; do
+  command -v "$c" >/dev/null || { echo "Missing command: $c"; exit 1; }
</file context>
Suggested change
REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat)
REQ_CMDS=(zypper systemctl)

echo 'WebInterface Yes' >> "$CUPSD_CONF"
fi

if ! grep -q '<Location />' "$CUPSD_CONF"; 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.

P1: Logic error: grep -q '<Location />' will match the existing <Location /> block present in default CUPS configurations (which typically has restrictive ACLs). Since the block already exists, this check passes and the script never adds Allow @LOCAL, leaving network access blocked (HTTP 403 Forbidden for LAN clients).

Additionally, the sed on line 37 only targets Listen localhost:631 — if the config uses 127.0.0.1:631 or *:631, it won't be commented out, and appending Port 631 will cause a port binding conflict that prevents CUPS from starting.

Consider using cupsctl --remote-any --share-printers which handles these cases safely and idempotently.

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 53:

<comment>Logic error: `grep -q '<Location />'` will match the existing `<Location />` block present in default CUPS configurations (which typically has restrictive ACLs). Since the block already exists, this check passes and the script never adds `Allow @LOCAL`, leaving network access blocked (HTTP 403 Forbidden for LAN clients).

Additionally, the `sed` on line 37 only targets `Listen localhost:631` — if the config uses `127.0.0.1:631` or `*:631`, it won't be commented out, and appending `Port 631` will cause a port binding conflict that prevents CUPS from starting.

Consider using `cupsctl --remote-any --share-printers` which handles these cases safely and idempotently.</comment>

<file context>
@@ -0,0 +1,118 @@
+  echo 'WebInterface Yes' >> "$CUPSD_CONF"
+fi
+
+if ! grep -q '<Location />' "$CUPSD_CONF"; then
+  cat >> "$CUPSD_CONF" <<'CUPSLOC'
+<Location />
</file context>

fi

echo "[3/8] Opening firewall for IPP + mDNS..."
firewall-cmd --permanent --add-service=ipp || true

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: Do not swallow firewall command failures; this can produce a false successful setup with broken network printing.

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 68:

<comment>Do not swallow firewall command failures; this can produce a false successful setup with broken network printing.</comment>

<file context>
@@ -0,0 +1,118 @@
+fi
+
+echo "[3/8] Opening firewall for IPP + mDNS..."
+firewall-cmd --permanent --add-service=ipp || true
+firewall-cmd --permanent --add-service=mdns || true
+firewall-cmd --reload || true
</file context>

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