Add setup script and README for Brother HL-L1222-V Android IPP print server on Raspberry Pi (openSUSE)#28
Conversation
Reviewer's GuideAdds an automated bash setup script and documentation to configure a Raspberry Pi 4B+ running openSUSE Tumbleweed as a CUPS-based IPP print server for a USB-connected Brother HL-L1222/HL-L1222-V, enabling Android Wi-Fi/Mopria printing with appropriate services, firewall rules, and printer queue configuration. Sequence diagram for Raspberry Pi print server setup script executionsequenceDiagram
participant User
participant SetupScript
participant Zypper
participant Systemd
participant CUPS
participant Avahi
participant Firewalld
participant Bluetooth
participant USBPrinter
User->>SetupScript: sudo bash setup_hl_l1222_android_print.sh
SetupScript->>SetupScript: Validate_root_and_required_commands
SetupScript->>Zypper: refresh
SetupScript->>Zypper: install cups cups-filters ghostscript avahi nss-mdns firewalld ipp-usb bluez bluez-tools
SetupScript->>Systemd: enable --now cups.service
Systemd-->>CUPS: start_cupsd
SetupScript->>Systemd: enable --now avahi-daemon.service
Systemd-->>Avahi: start_avahi
SetupScript->>Systemd: enable --now firewalld.service
Systemd-->>Firewalld: start_firewalld
SetupScript->>Systemd: enable --now bluetooth.service
Systemd-->>Bluetooth: start_bluetooth
SetupScript->>CUPS: update_cupsd_conf_for_network_listening_and_sharing
SetupScript->>Firewalld: add_service ipp
SetupScript->>Firewalld: add_service mdns
SetupScript->>Firewalld: reload
USBPrinter-->>SetupScript: lpinfo -v output (usb Brother URI)
SetupScript->>CUPS: lpadmin -p Brother_HL_L1222_V -E -v usb_uri -m everywhere
SetupScript->>CUPS: lpadmin options sides=one-sided media=A4 printer-is-shared=true
SetupScript->>Systemd: restart cups.service
Systemd-->>CUPS: restart_cupsd
SetupScript->>CUPS: lp testprint to Brother_HL_L1222_V
CUPS->>USBPrinter: send_test_page
SetupScript-->>User: show lpstat -t and IPP URL for Android
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a setup script and documentation for a Raspberry Pi print server. Key feedback includes switching to the brlaser driver, removing ipp-usb to avoid conflicts, and using cupsctl for more reliable configuration. Additionally, the test print file path should be corrected for compatibility.
| # practical network printing path and optional USB OTG fallback guidance. | ||
|
|
||
| PRINTER_NAME="Brother_HL_L1222_V" | ||
| PPD_MODEL="everywhere" |
There was a problem hiding this comment.
The Brother HL-L1222-V is a GDI (host-based) printer and does not support the IPP Everywhere protocol. Using everywhere as the PPD model will likely result in "Filter failed" errors or no output. This printer typically requires the brlaser driver.
| PPD_MODEL="everywhere" | |
| PPD_MODEL="drv:///brlaser.drv/brl1222v.ppd" |
| 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 |
There was a problem hiding this comment.
Manually editing cupsd.conf with sed and echo is brittle and not idempotent. Specifically, the check if ! grep -q '<Location />' will almost always fail on a standard CUPS installation because that block usually exists by default, meaning the Allow @LOCAL directive will never be added. This will prevent network clients from accessing the printer. Using cupsctl is the recommended, robust way to enable sharing and remote access.
| 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 | |
| cupsctl --remote-any --share-printers --user-interface |
| zypper --non-interactive refresh | ||
| zypper --non-interactive install \ | ||
| cups cups-filters ghostscript avahi nss-mdns \ | ||
| firewalld ipp-usb bluez bluez-tools |
There was a problem hiding this comment.
The ipp-usb package can conflict with the standard CUPS USB backend for printers that do not natively support IPP-over-USB (like the HL-L1222). The ipp-usb daemon may claim the USB interface, preventing CUPS from accessing it. Additionally, cups-drivers-brlaser should be included to provide the necessary driver.
| firewalld ipp-usb bluez bluez-tools | |
| firewalld cups-drivers-brlaser bluez bluez-tools |
| systemctl restart cups.service | ||
|
|
||
| echo "[7/8] Printing a test page..." | ||
| lp -d "$PRINTER_NAME" /usr/share/cups/data/testprint || true |
There was a problem hiding this comment.
The path /usr/share/cups/data/testprint may not exist or may require a specific file extension (like .ps or .pdf) depending on the CUPS version and installed filters. If this file is missing, the test print will fail.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Consider using
cupsctl(e.g.,cupsctl --share-printers --remote-any --web-interface=yes) or a drop-in config snippet instead of directly editing/etc/cups/cupsd.conf, and at minimum create a backup of the original file before mutating it to make the script safer and more reversible. - The IP detection
hostname -I | awk '{print $1}'may pick an unexpected interface on devices with multiple addresses (VPNs, Docker, etc.); consider filtering for a specific interface or prompting the user to choose from the available IPs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using `cupsctl` (e.g., `cupsctl --share-printers --remote-any --web-interface=yes`) or a drop-in config snippet instead of directly editing `/etc/cups/cupsd.conf`, and at minimum create a backup of the original file before mutating it to make the script safer and more reversible.
- The IP detection `hostname -I | awk '{print $1}'` may pick an unexpected interface on devices with multiple addresses (VPNs, Docker, etc.); consider filtering for a specific interface or prompting the user to choose from the available IPs.
## Individual Comments
### Comment 1
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="67-70" />
<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):** Silencing firewall-cmd failures may hide real configuration problems.
Because every `firewall-cmd` call is suffixed with `|| true`, failures (e.g., firewalld not running, missing zones/services, or permission issues) are silently ignored and the script still appears to succeed, even if printing won’t work. Since you enable `firewalld.service` earlier, consider treating at least the first `firewall-cmd` invocation as mandatory and failing with a clear error if it doesn’t succeed, while leaving later calls non-fatal if desired.
```suggestion
echo "[3/8] Opening firewall for IPP + mDNS..."
if ! firewall-cmd --permanent --add-service=ipp; then
echo "Error: Failed to configure firewall for IPP via firewall-cmd."
echo "Ensure firewalld is installed, running, and that you have sufficient permissions, then re-run this script."
exit 1
fi
firewall-cmd --permanent --add-service=mdns || true
firewall-cmd --reload || true
```
</issue_to_address>
### Comment 2
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="105-106" />
<code_context>
+echo "[8/8] Final status"
+lpstat -t
+
+IP_ADDR="$(hostname -I | awk '{print $1}')"
+cat <<MSG
+
+Setup complete.
+
+Android printing method (recommended):
+1) Connect phone to same Wi-Fi as Raspberry Pi.
+2) Ensure Mopria Print Service is enabled on Android.
+3) Add/find printer: ipp://$IP_ADDR/printers/$PRINTER_NAME
+
+Bluetooth note:
</code_context>
<issue_to_address>
**suggestion:** Using `hostname -I | awk '{print $1}'` may select an unexpected or unusable IP address.
On hosts with multiple interfaces (Wi‑Fi, Ethernet, VPNs, docker, etc.), `hostname -I` often returns several addresses, and the first one may be unreachable from Android (e.g., link-local or internal). It would be more robust to target a specific interface (e.g., via `ip -4 addr show <iface>` and filtering out link-local addresses) or, at minimum, detect multiple candidates and prompt the user to choose the correct IP.
```suggestion
# Detect candidate IPv4 addresses (non-loopback, global scope)
CANDIDATE_IPS=()
if command -v ip >/dev/null 2>&1; then
# Use `ip` if available for more reliable address detection
while IFS= read -r ip; do
CANDIDATE_IPS+=("$ip")
done < <(ip -4 addr show scope global | awk '/inet / {print $2}' | cut -d/ -f1)
fi
# Fallback to hostname -I if `ip` produced nothing
if [ "${#CANDIDATE_IPS[@]}" -eq 0 ]; then
for ip in $(hostname -I 2>/dev/null); do
# skip obvious loopback addresses just in case
case "$ip" in
127.*) continue ;;
esac
CANDIDATE_IPS+=("$ip")
done
fi
# If we still have nothing, leave IP_ADDR empty
IP_ADDR=""
if [ "${#CANDIDATE_IPS[@]}" -eq 1 ]; then
IP_ADDR="${CANDIDATE_IPS[0]}"
elif [ "${#CANDIDATE_IPS[@]}" -gt 1 ]; then
echo
echo "Multiple candidate IP addresses were detected for this Raspberry Pi:"
i=1
for ip in "${CANDIDATE_IPS[@]}"; do
echo " [$i] $ip"
i=$((i+1))
done
echo
# prompt user to choose; default to first if they just press Enter
read -rp "Select the IP address that your Android device can reach [1]: " choice
case "$choice" in
""|*[!0-9]*)
IP_ADDR="${CANDIDATE_IPS[0]}"
;;
*)
idx=$((choice-1))
if [ "$idx" -ge 0 ] && [ "$idx" -lt "${#CANDIDATE_IPS[@]}" ]; then
IP_ADDR="${CANDIDATE_IPS[$idx]}"
else
IP_ADDR="${CANDIDATE_IPS[0]}"
fi
;;
esac
fi
cat <<MSG
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| 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 |
There was a problem hiding this comment.
suggestion (bug_risk): Silencing firewall-cmd failures may hide real configuration problems.
Because every firewall-cmd call is suffixed with || true, failures (e.g., firewalld not running, missing zones/services, or permission issues) are silently ignored and the script still appears to succeed, even if printing won’t work. Since you enable firewalld.service earlier, consider treating at least the first firewall-cmd invocation as mandatory and failing with a clear error if it doesn’t succeed, while leaving later calls non-fatal if desired.
| 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 "[3/8] Opening firewall for IPP + mDNS..." | |
| if ! firewall-cmd --permanent --add-service=ipp; then | |
| echo "Error: Failed to configure firewall for IPP via firewall-cmd." | |
| echo "Ensure firewalld is installed, running, and that you have sufficient permissions, then re-run this script." | |
| exit 1 | |
| fi | |
| firewall-cmd --permanent --add-service=mdns || true | |
| firewall-cmd --reload || true |
| IP_ADDR="$(hostname -I | awk '{print $1}')" | ||
| cat <<MSG |
There was a problem hiding this comment.
suggestion: Using hostname -I | awk '{print $1}' may select an unexpected or unusable IP address.
On hosts with multiple interfaces (Wi‑Fi, Ethernet, VPNs, docker, etc.), hostname -I often returns several addresses, and the first one may be unreachable from Android (e.g., link-local or internal). It would be more robust to target a specific interface (e.g., via ip -4 addr show <iface> and filtering out link-local addresses) or, at minimum, detect multiple candidates and prompt the user to choose the correct IP.
| IP_ADDR="$(hostname -I | awk '{print $1}')" | |
| cat <<MSG | |
| # Detect candidate IPv4 addresses (non-loopback, global scope) | |
| CANDIDATE_IPS=() | |
| if command -v ip >/dev/null 2>&1; then | |
| # Use `ip` if available for more reliable address detection | |
| while IFS= read -r ip; do | |
| CANDIDATE_IPS+=("$ip") | |
| done < <(ip -4 addr show scope global | awk '/inet / {print $2}' | cut -d/ -f1) | |
| fi | |
| # Fallback to hostname -I if `ip` produced nothing | |
| if [ "${#CANDIDATE_IPS[@]}" -eq 0 ]; then | |
| for ip in $(hostname -I 2>/dev/null); do | |
| # skip obvious loopback addresses just in case | |
| case "$ip" in | |
| 127.*) continue ;; | |
| esac | |
| CANDIDATE_IPS+=("$ip") | |
| done | |
| fi | |
| # If we still have nothing, leave IP_ADDR empty | |
| IP_ADDR="" | |
| if [ "${#CANDIDATE_IPS[@]}" -eq 1 ]; then | |
| IP_ADDR="${CANDIDATE_IPS[0]}" | |
| elif [ "${#CANDIDATE_IPS[@]}" -gt 1 ]; then | |
| echo | |
| echo "Multiple candidate IP addresses were detected for this Raspberry Pi:" | |
| i=1 | |
| for ip in "${CANDIDATE_IPS[@]}"; do | |
| echo " [$i] $ip" | |
| i=$((i+1)) | |
| done | |
| echo | |
| # prompt user to choose; default to first if they just press Enter | |
| read -rp "Select the IP address that your Android device can reach [1]: " choice | |
| case "$choice" in | |
| ""|*[!0-9]*) | |
| IP_ADDR="${CANDIDATE_IPS[0]}" | |
| ;; | |
| *) | |
| idx=$((choice-1)) | |
| if [ "$idx" -ge 0 ] && [ "$idx" -lt "${#CANDIDATE_IPS[@]}" ]; then | |
| IP_ADDR="${CANDIDATE_IPS[$idx]}" | |
| else | |
| IP_ADDR="${CANDIDATE_IPS[0]}" | |
| fi | |
| ;; | |
| esac | |
| fi | |
| cat <<MSG |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| 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 |
There was a problem hiding this comment.
🔴 CUPS commands checked before CUPS package is installed, causing script failure on fresh systems
Lines 17-20 check for lpinfo, lpadmin, and lpstat via command -v, but these are all provided by the cups package which isn't installed until lines 24-26. On a fresh openSUSE system (the target environment), these commands won't exist, and the script will exit with "Missing command: lpinfo" before it ever reaches the zypper install cups step. This completely prevents the script from running on any system that doesn't already have CUPS — which is exactly the system this setup script is meant to configure.
| 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 | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| 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 |
There was a problem hiding this comment.
🔴 Existing <Location /> block in default CUPS config prevents Allow @LOCAL from being added
Line 53 uses grep -q '<Location />' to decide whether to append the <Location /> block with Allow @LOCAL. However, a default CUPS installation already ships a <Location /> block that restricts access to localhost only (no Allow @LOCAL). The grep will find this existing block and skip adding the Allow @LOCAL directive, meaning Android clients on the LAN will be denied access to the printer — defeating the entire purpose of the script.
Default CUPS cupsd.conf typically contains
<Location />
Order allow,deny
</Location>
Since this block exists, the grep matches, the script's Allow @LOCAL block is never appended, and network clients are denied.
Prompt for agents
The script checks whether `<Location />` exists in cupsd.conf and only appends the Allow @LOCAL block if it doesn't. But the default CUPS config already has a `<Location />` block (restricting access to localhost), so the check always matches and the Allow @LOCAL directive is never added.
The fix needs to handle both cases:
1. If `<Location />` exists but doesn't contain `Allow @LOCAL`, inject the Allow @LOCAL directive into the existing block.
2. If `<Location />` doesn't exist at all, append the full block.
Similar logic is needed for `<Location /admin>`.
One approach: use sed to find the existing `<Location />` block and add `Allow @LOCAL` inside it if not already present. For example:
sed -i '/<Location \/>/,/<\/Location>/ { /Order allow,deny/a\ Allow @LOCAL' } cupsd.conf
Alternatively, replace the existing `<Location />` blocks entirely with the desired configuration using a more robust sed or awk transformation. The key file is /etc/cups/cupsd.conf and the functions involved are the cupsd.conf configuration section around line 53 of setup_hl_l1222_android_print.sh.
Was this helpful? React with 👍 or 👎 to provide feedback.
| 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 |
There was a problem hiding this comment.
🚩 cupsd.conf modifications are append-only and not idempotent across re-runs
The cupsd.conf modifications at lines 36-65 use a pattern of grep -q to check existence and then echo >> "$CUPSD_CONF" to append. If the script is run multiple times (e.g., after a failed USB detection), and between runs the config already has Port 631 from a previous run, the guards prevent duplication — which is good. However, appending directives like Port 631, Browsing Yes, etc. to the END of the file may cause issues if the default config has conflicting directives earlier (e.g., Listen localhost:631 and Browsing No). CUPS uses the last-wins semantic for some directives but not all. The sed -i 's/^Listen localhost:631/# &/' on line 37 partially addresses this for the Listen directive, but similar conflicting directives (Browsing No, DefaultShared No) are not commented out before appending replacements.
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}')" |
There was a problem hiding this comment.
📝 Info: USB URI detection awk pattern may not match all lpinfo output formats
Line 73 uses awk '/^direct usb:\/\/Brother\//{print $2; exit}' to extract the USB URI from lpinfo -v. The regex assumes the output starts with direct usb://Brother/ with exactly one space between direct and usb://. Some CUPS versions pad the scheme class field with multiple spaces or tabs. If the whitespace differs, the regex won't match even though the printer is connected. The $2 field extraction via awk's default whitespace splitting would still work for extracting the URI, but the regex anchor ^direct usb:// is fragile. A safer pattern might be /usb:\/\/Brother\//{print $2; exit} without anchoring to ^direct . This wasn't flagged as a bug because the standard lpinfo -v format on openSUSE with CUPS 2.x typically uses a single space, but it's worth noting.
Was this helpful? React with 👍 or 👎 to provide feedback.
| echo "[7/8] Printing a test page..." | ||
| lp -d "$PRINTER_NAME" /usr/share/cups/data/testprint || true |
There was a problem hiding this comment.
📝 Info: Test page is printed unconditionally on every run
Line 100 prints a test page (lp -d "$PRINTER_NAME" /usr/share/cups/data/testprint || true) every time the script is run. If an operator re-runs the script (e.g., to fix config), a physical page will be printed each time, wasting paper. This is not a bug per se, but could be surprising for re-runs. A --no-test flag or a prompt would be a more user-friendly approach.
Was this helpful? React with 👍 or 👎 to provide feedback.
| USB_URI="$(lpinfo -v | awk '/^direct usb:\/\/Brother\//{print $2; exit}')" | ||
| if [[ -z "$USB_URI" ]]; then | ||
| echo "No Brother USB URI auto-detected. Available devices:" | ||
| lpinfo -v | ||
| echo "Connect the printer via USB and re-run this script." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
📝 Info: Script uses set -euo pipefail but pipes lpinfo -v through awk without handling pipefail
Line 73: USB_URI="$(lpinfo -v | awk '...')" — with pipefail enabled, if lpinfo -v fails (e.g., CUPS service not fully started yet after the enable --now on line 29), the pipeline exit status will be non-zero and set -e will terminate the script before reaching the friendlier error message at lines 74-78. The || true pattern used elsewhere (e.g., line 100) is not applied here, which is actually reasonable since a failed lpinfo is a real error, but the error message won't be the script's helpful diagnostic — it'll be an abrupt exit. This is a minor UX concern, not a correctness bug.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5e8e49d51
ℹ️ 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".
| 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 |
There was a problem hiding this comment.
The preflight loop checks lpinfo, lpadmin, and lpstat before the script installs cups, so a fresh target host without CUPS exits immediately at "Missing command" and never reaches the installation step. This makes the setup non-bootstrapping in exactly the environment this script is meant to provision.
Useful? React with 👍 / 👎.
| if ! grep -q '<Location />' "$CUPSD_CONF"; then | ||
| cat >> "$CUPSD_CONF" <<'CUPSLOC' |
There was a problem hiding this comment.
The LAN access block is only appended when <Location /> is absent, but standard cupsd.conf files already contain that section. In that common case this script does not relax the existing ACLs for local-network clients, so CUPS may still reject remote Android IPP requests even though Port 631 and sharing are enabled.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
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: The preflight command check requires CUPS tools before they are installed, which can make the setup fail immediately on clean hosts.</violation>
<violation number="2" location="scripts/setup_hl_l1222_android_print.sh:41">
P1: The script appends `Browsing Yes` and `DefaultShared Yes` but does not comment out the conflicting `Browsing No` / `DefaultShared No` directives that typically exist in the default `cupsd.conf`. While `Listen localhost:631` is correctly commented out via `sed` before appending `Port 631`, the same treatment is missing here. CUPS may use the earlier conflicting value, leaving sharing and browsing disabled.</violation>
<violation number="3" location="scripts/setup_hl_l1222_android_print.sh:53">
P0: Default CUPS `cupsd.conf` already contains a `<Location />` block (restricting access to localhost). Since `grep -q '<Location />'` finds the existing block, the `Allow @LOCAL` directive is never appended, so Android LAN clients will still be denied access—defeating the purpose of this script.
The fix should inject `Allow @LOCAL` into the existing `<Location />` block if it doesn't already contain it, or use `cupsctl --remote-any --share-printers` which handles this robustly.</violation>
<violation number="4" location="scripts/setup_hl_l1222_android_print.sh:68">
P2: Do not swallow firewall-cmd failures; this can report success while network printing is still blocked.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| echo 'WebInterface Yes' >> "$CUPSD_CONF" | ||
| fi | ||
|
|
||
| if ! grep -q '<Location />' "$CUPSD_CONF"; then |
There was a problem hiding this comment.
P0: Default CUPS cupsd.conf already contains a <Location /> block (restricting access to localhost). Since grep -q '<Location />' finds the existing block, the Allow @LOCAL directive is never appended, so Android LAN clients will still be denied access—defeating the purpose of this script.
The fix should inject Allow @LOCAL into the existing <Location /> block if it doesn't already contain it, or use cupsctl --remote-any --share-printers which handles this robustly.
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>Default CUPS `cupsd.conf` already contains a `<Location />` block (restricting access to localhost). Since `grep -q '<Location />'` finds the existing block, the `Allow @LOCAL` directive is never appended, so Android LAN clients will still be denied access—defeating the purpose of this script.
The fix should inject `Allow @LOCAL` into the existing `<Location />` block if it doesn't already contain it, or use `cupsctl --remote-any --share-printers` which handles this robustly.</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>
| echo 'Port 631' >> "$CUPSD_CONF" | ||
| fi | ||
|
|
||
| if ! grep -q '^Browsing Yes' "$CUPSD_CONF"; then |
There was a problem hiding this comment.
P1: The script appends Browsing Yes and DefaultShared Yes but does not comment out the conflicting Browsing No / DefaultShared No directives that typically exist in the default cupsd.conf. While Listen localhost:631 is correctly commented out via sed before appending Port 631, the same treatment is missing here. CUPS may use the earlier conflicting value, leaving sharing and browsing disabled.
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 41:
<comment>The script appends `Browsing Yes` and `DefaultShared Yes` but does not comment out the conflicting `Browsing No` / `DefaultShared No` directives that typically exist in the default `cupsd.conf`. While `Listen localhost:631` is correctly commented out via `sed` before appending `Port 631`, the same treatment is missing here. CUPS may use the earlier conflicting value, leaving sharing and browsing disabled.</comment>
<file context>
@@ -0,0 +1,118 @@
+ echo 'Port 631' >> "$CUPSD_CONF"
+fi
+
+if ! grep -q '^Browsing Yes' "$CUPSD_CONF"; then
+ echo 'Browsing Yes' >> "$CUPSD_CONF"
+fi
</file context>
| exit 1 | ||
| fi | ||
|
|
||
| REQ_CMDS=(zypper systemctl lpinfo lpadmin lpstat) |
There was a problem hiding this comment.
P1: The preflight command check requires CUPS tools before they are installed, which can make the setup fail immediately on clean hosts.
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>The preflight command check requires CUPS tools before they are installed, which can make the setup fail immediately on clean hosts.</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>
| fi | ||
|
|
||
| echo "[3/8] Opening firewall for IPP + mDNS..." | ||
| firewall-cmd --permanent --add-service=ipp || true |
There was a problem hiding this comment.
P2: Do not swallow firewall-cmd failures; this can report success while network printing is still blocked.
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-cmd failures; this can report success while network printing is still blocked.</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>
1 participant
Motivation
Description
scripts/setup_hl_l1222_android_print.shthat installs required packages withzypper, enables services viasystemctl, and configures CUPS, Avahi, Firewalld, and Bluetooth./etc/cups/cupsd.confto enable network listening, sharing, and web interface, opens firewall rules forippandmdnsviafirewall-cmd, and attempts to auto-detect the Brother USB URI usinglpinfo.lpadminusing theeverywheremodel, sets default options (one-sided, A4), enables sharing, restarts CUPS, and issues a test print andlpstat -tstatus.README.mdwith usage instructions, the recommended IPPipp://<raspberrypi-ip>/printers/Brother_HL_L1222_Vworkflow, and notes about Bluetooth and driver options.Testing
Codex Task
Summary by Sourcery
Add a setup script and documentation for configuring a Raspberry Pi (openSUSE Tumbleweed) as an IPP/Mopria-compatible print server for a USB-connected Brother HL-L1222/HL-L1222-V printer.
New Features:
Documentation:
Summary by cubic
Adds a one-command setup to turn a Raspberry Pi 4B+ (openSUSE Tumbleweed) into a CUPS-backed IPP print server for a USB Brother HL‑L1222/HL‑L1222‑V, enabling Android Wi‑Fi printing via Mopria. Updates the README with run steps, the IPP URI, and Bluetooth caveats.
scripts/setup_hl_l1222_android_print.shto auto-configure the print server.zypper/systemctl:cups,cups-filters,ghostscript,avahi,nss-mdns,firewalld,ipp-usb,bluez,bluez-tools./etc/cups/cupsd.conf) to listen on port 631, enable sharing and web UI, allow@LOCAL; opens firewall forippandmdns.Brother_HL_L1222_Vqueue using the detected USB URI andeverywheremodel; setsA4and one-sided; shares the printer; prints a test page and shows status.README.mdwith usage and Android setup, includingipp://<raspberrypi-ip>/printers/Brother_HL_L1222_Vand a note that Bluetooth printing is not reliably supported.Written for commit a5e8e49. Summary will update on new commits. Review in cubic