Add mobile printing tutorial and setup script for Brother HL‑L1222 (openSUSE/CUPS)#33
Conversation
Reviewer's GuideAdds a comprehensive iPhone-focused usage/troubleshooting README for a Raspberry Pi + openSUSE CUPS Brother HL‑L1222 print server and introduces a full-featured Bash setup script that configures CUPS, firewall, discovery, a hotfolder watcher, and an optional upload web service for mobile printing. Sequence diagram for upload fallback printing flowsequenceDiagram
actor iPhone
participant Browser as iPhone_Safari
participant UploadSvc as hll1222_upload_service
participant Hotfolder as Hotfolder_directory
participant Watcher as hll1222_hotfolder_service
participant CUPS as CUPS_server
participant Printer as Brother_HL_L1222_USB
iPhone->>Browser: Open http://pi_ip:8088/
Browser->>UploadSvc: GET /
UploadSvc-->>Browser: HTML upload_form
iPhone->>Browser: Select_file_and_submit
Browser->>UploadSvc: POST /upload (multipart_file)
UploadSvc->>UploadSvc: Validate_size_and_extension
UploadSvc->>Hotfolder: Write_file_to_hotfolder
UploadSvc-->>Browser: 200 OK (Uploaded_successfully)
Watcher->>Hotfolder: inotifywait for_new_files
Hotfolder-->>Watcher: New_file_created
Watcher->>CUPS: lp -d QUEUE_NAME file
CUPS->>Printer: Send_print_job_over_USB
Printer-->>CUPS: Job_completed_status
CUPS-->>Watcher: Job_done
Watcher->>Hotfolder: Remove_processed_file
Flow diagram for setup_hl_l1222_android_print.sh executionflowchart TD
A[Start_script
parse_args] --> B[Validate_required_commands
sudo zypper lpinfo lpadmin lpstat]
B --> C{--status_flag?}
C -- yes --> S[Run_status_report
usb_devices_CUPS_devices_models
exit]
C -- no --> D[Install_dependencies
cups cups_client filters
avahi firewalld usbutils
python3 python3_python_multipart
optional_printer_driver_brlaser
optional_Bluetooth]
D --> E[Enable_core_services
cups avahi_daemon firewalld
optional_bluetooth]
E --> F[Configure_firewall
add_ipp mdns and_optional_8088]
F --> G[Confirm_and_backup
/etc/cups/cupsd.conf]
G --> H[Configure_CUPS_sharing
cupsctl --share-printers --remote-any
ensure_WebInterface_Browsing_dnssd]
H --> I[Detect_USB_DEVICE_URI
lpinfo -v filter_Brother]
I --> J[Select_MODEL_NAME
prefer_brlaser_Brother
else_everywhere
else_generic_mono_laser]
J --> K{Queue_exists?}
K -- yes --> L[Confirm_and_delete_existing_queue]
L --> M[Create_new_queue
lpadmin -p QUEUE_NAME -v DEVICE_URI -m MODEL_NAME
set_defaults_share_enable_accept]
K -- no --> M
M --> N[Restart_CUPS]
N --> O[Install_hotfolder_service
create_script_dir_and_hotfolder
chmod_0777_hotfolder
install_watch_print.sh
create_systemd_hll1222_hotfolder]
O --> P[Start_and_enable_hotfolder_service]
P --> Q{ENABLE_UPLOAD?}
Q -- no --> U[Print_summary
show_IPP_URL_and_optional_notes]
Q -- yes --> R{python3_python_multipart_available?}
R -- no --> T[Skip_upload_service
warn_user]
R -- yes --> V[Install_upload_server
upload_server.py
systemd_hll1222_upload]
V --> W[Start_and_enable_upload_service]
T --> U
W --> U
U --> Z[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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 left some high level feedback:
- The
--statusflag is effectively broken becauseSTATUS_ONLYis declared as a local insidemainafterparse_argsruns, so the value set inparse_argsis never seen; consider either passing it back explicitly or not shadowing it with a local. - In
print_summary, the lineecho " "$LPINFO_BIN" -m | grep ...ends up executinglpinfopiped togrepinstead of printing the command as a string; wrap the whole command in a single quoted string if you intend to show the command rather than run it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `--status` flag is effectively broken because `STATUS_ONLY` is declared as a local inside `main` after `parse_args` runs, so the value set in `parse_args` is never seen; consider either passing it back explicitly or not shadowing it with a local.
- In `print_summary`, the line `echo " "$LPINFO_BIN" -m | grep ...` ends up executing `lpinfo` piped to `grep` instead of printing the command as a string; wrap the whole command in a single quoted string if you intend to show the command rather than run it.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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: ff9701f52b
ℹ️ 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".
| if ! sudo zypper --non-interactive install --no-recommends \ | ||
| cups cups-client cups-filters ghostscript \ | ||
| avahi nss-mdns firewalld usbutils python3 python3-python-multipart; then | ||
| err "Failed to install required base packages." |
There was a problem hiding this comment.
The base dependency install unconditionally requires python3-python-multipart, so the script aborts even when --enable-upload is not set. This breaks the default path (ENABLE_UPLOAD="no") on systems/repositories where that package is unavailable, despite later logic already treating upload as optional and capable of being skipped. Move this package out of the mandatory install set (or gate it behind ENABLE_UPLOAD) so core IPP/CUPS setup can still complete.
Useful? React with 👍 / 👎.
| Use an iPhone on the same Wi-Fi network to print through your Raspberry Pi 4B+ openSUSE CUPS server connected by USB to a Brother HL-L1222/HL-L1222-V. | ||
|
|
||
| This tutorial assumes your setup script has already run successfully and created: | ||
| - CUPS queue: `impressora_brother` |
There was a problem hiding this comment.
The README states that running the setup script creates queue impressora_brother, but the script’s default is Brother_HL_L1222_V unless --queue is passed. Following the documented commands as written will cause queue-specific checks and test prints to target a non-existent queue on default installs, which makes the troubleshooting flow unreliable.
Useful? React with 👍 / 👎.
![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 comprehensive setup for a Raspberry Pi-based print server for Brother HL-L1222 printers, including a configuration script and an iPhone-specific tutorial. Feedback highlights several critical issues in the script, including API mismatches in the Python upload service that would cause runtime crashes, insecure file permissions for the hotfolder, and logic errors in the file cleanup process. Additionally, an inconsistency between the tutorial documentation and the script's default queue name was identified.
| Use an iPhone on the same Wi-Fi network to print through your Raspberry Pi 4B+ openSUSE CUPS server connected by USB to a Brother HL-L1222/HL-L1222-V. | ||
|
|
||
| This tutorial assumes your setup script has already run successfully and created: | ||
| - CUPS queue: `impressora_brother` |
There was a problem hiding this comment.
The queue name impressora_brother mentioned here and throughout the tutorial does not match the default Brother_HL_L1222_V defined in the setup script (line 24). This inconsistency will cause confusion for users following the tutorial after running the script.
| - CUPS queue: `impressora_brother` | |
| - CUPS queue: `Brother_HL_L1222_V` |
| "CONTENT_LENGTH": str(clen), | ||
| "wsgi.input": self.rfile, | ||
| } | ||
| _form, files = parse_form_data(env) |
There was a problem hiding this comment.
The python3-python-multipart package installed in line 193 (which provides the multipart module) does not have a parse_form_data function that accepts a single dictionary argument. This will cause a runtime error. You likely intended to use the multipart library (by @defnull) or need to update the code to use the python-multipart API (e.g., BaseParser).
| with open(dst, "wb") as f: | ||
| f.write(item.raw) |
There was a problem hiding this comment.
The attribute item.raw is likely incorrect for the multipart library. Depending on the library version, it is usually item.file.read() or item.value. Using an undefined attribute will cause the upload service to crash upon receiving a file.
| exit 1 | ||
| fi | ||
| # Keep folder writable for upload service and watcher workflows. | ||
| if ! sudo chmod 0777 "$HOTFOLDER"; then |
There was a problem hiding this comment.
Setting the hotfolder to 0777 is a security risk. Since the script already sets the ownership to lp:lp (line 391) and the upload service runs as the lp user (lines 594-595), 0775 is sufficient to allow the necessary access while maintaining better security.
| if ! sudo chmod 0777 "$HOTFOLDER"; then | |
| if ! sudo chmod 0775 "$HOTFOLDER"; then |
| lp -d "$QUEUE" -o fit-to-page "$file" || true | ||
| rm -f "$file" || true |
There was a problem hiding this comment.
The current logic deletes the file even if the lp command fails. It is better to only remove the file if the print job was successfully submitted to the queue.
| lp -d "$QUEUE" -o fit-to-page "$file" || true | |
| rm -f "$file" || true | |
| lp -d "$QUEUE" -o fit-to-page "$file" && rm -f "$file" || true |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| systemctl status firewalld --no-pager | ||
| lpstat -t | ||
| ``` | ||
|
|
||
| If you enabled upload mode: | ||
|
|
||
| ```bash | ||
| systemctl status hll1222-upload --no-pager |
There was a problem hiding this comment.
🔴 README tutorial references wrong CUPS queue name, mismatching the script default
The README documentation consistently references the queue name impressora_brother (at README.md:7, README.md:61-62, README.md:68, README.md:81, README.md:201, README.md:221), but the setup script's default QUEUE_NAME at scripts/setup_hl_l1222_android_print.sh:24 is "Brother_HL_L1222_V". A user who runs the script with defaults gets a queue named Brother_HL_L1222_V, and then every command in the README tutorial (lpstat -p impressora_brother, lp -d impressora_brother, etc.) will fail because the queue impressora_brother does not exist.
Prompt for agents
The README tutorial references queue name `impressora_brother` throughout, but the setup script (scripts/setup_hl_l1222_android_print.sh:24) defaults to `Brother_HL_L1222_V`. All tutorial commands that reference `impressora_brother` need to be updated to match the script's actual default queue name `Brother_HL_L1222_V`, or the script default should be changed to `impressora_brother`. Affected README locations: lines 7, 61, 62, 68, 81, 201, and 221.
Was this helpful? React with 👍 or 👎 to provide feedback.
| exit 1 | ||
| fi | ||
|
|
||
| TMP_DIR="" |
There was a problem hiding this comment.
🟡 Temp directories from mktemp are leaked because TMP_DIR is cleared before cleanup runs
Both install_hotfolder_service (line 401) and install_upload_service_optional (line 488) create temp directories via mktemp -d and store the path in TMP_DIR. After using the temp files, each function sets TMP_DIR="" (lines 472 and 623) without first removing the directory. The cleanup trap (scripts/setup_hl_l1222_android_print.sh:62-66) checks TMP_DIR on exit, but since it's empty by then, neither temp directory is ever removed. The fix is to rm -rf "$TMP_DIR" before clearing the variable.
| TMP_DIR="" | |
| rm -rf "$TMP_DIR" | |
| TMP_DIR="" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| exit 1 | ||
| fi | ||
|
|
||
| TMP_DIR="" |
There was a problem hiding this comment.
🟡 Second temp directory also leaked in install_upload_service_optional
Same issue as the hotfolder function: TMP_DIR is set to empty string at line 623 without first removing the temp directory created at line 488. This is the second location of the same resource leak pattern.
| TMP_DIR="" | |
| rm -rf "$TMP_DIR" | |
| TMP_DIR="" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| dst = hot / f"{Path(safe).stem}_{i}{ext}" | ||
| i += 1 | ||
| with open(dst, "wb") as f: | ||
| f.write(item.raw) |
There was a problem hiding this comment.
🔴 Upload server uses item.raw which is not a valid attribute on python-multipart File objects
The embedded Python upload server at line 563 writes file content using item.raw, but the File objects returned by python-multipart's parse_form_data do not have a .raw attribute. The standard attributes are .value (bytes content) or .file (file-like object). At runtime, this will raise AttributeError, causing every file upload to fail with a 500 error. The ThreadingHTTPServer will catch the exception per-request so the server stays up, but the upload feature is non-functional.
Likely correct usage
Replace f.write(item.raw) with f.write(item.value) or f.write(item.file.read()).
| f.write(item.raw) | |
| f.write(item.value) |
Was this helpful? React with 👍 or 👎 to provide feedback.
| self.send_body(400, "Missing file") | ||
| return | ||
| item = files["file"] | ||
| name = Path(getattr(item, "filename", "")).name |
There was a problem hiding this comment.
🚩 Upload server getattr(item, "filename", "") may use wrong attribute name
At scripts/setup_hl_l1222_android_print.sh:548, the code uses getattr(item, "filename", "") to get the uploaded file's name. In the python-multipart library, the File object returned by parse_form_data typically exposes the filename as .file_name (with underscore), not .filename. If the attribute name is wrong, getattr silently returns "", Path("").name evaluates to "", and every upload is rejected with 400 "Missing filename" — a silent failure with a misleading error message. This is closely related to the .raw bug already reported but affects a different code path. Needs verification against the specific python-multipart version packaged for openSUSE.
Was this helpful? React with 👍 or 👎 to provide feedback.
| safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160] | ||
| dst = hot / safe |
There was a problem hiding this comment.
📝 Info: Filename sanitization truncation can mangle file extension
At scripts/setup_hl_l1222_android_print.sh:556, the sanitized filename is truncated to 160 characters: safe = "...")[:160]. If the original filename is long enough that truncation cuts into the extension (e.g., a 157-char name + .pdf becomes 160 chars ending in .pd), the file written to the hotfolder at line 557 will have a mangled extension. The hotfolder watcher (scripts/setup_hl_l1222_android_print.sh:419-420) only processes files matching specific extensions (.pdf, .jpg, etc.), so the truncated file would sit in the hotfolder forever, unprintable. The user would see "Uploaded successfully" but the file would never print. This is an extreme edge case (requires filenames >156 chars) but results in a silent failure.
Was this helpful? React with 👍 or 👎 to provide feedback.
| local STATUS_ONLY="no" | ||
| parse_args "$@" |
There was a problem hiding this comment.
📝 Info: parse_args STATUS_ONLY variable scoping works correctly due to bash dynamic scope
At scripts/setup_hl_l1222_android_print.sh:657, main declares local STATUS_ONLY="no", and then parse_args at line 147 assigns STATUS_ONLY="yes" without its own local declaration. This initially looks like a scoping bug where parse_args might create a separate global instead of modifying main's local. However, bash uses dynamic scoping for local variables: a called function that doesn't declare its own local of the same name will read/write the caller's local variable. So parse_args correctly modifies main's STATUS_ONLY. This is not a bug, just a potentially confusing pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
| --queue) | ||
| QUEUE_NAME="$2" | ||
| shift 2 | ||
| ;; | ||
| --device) | ||
| DEVICE_URI="$2" | ||
| shift 2 | ||
| ;; | ||
| --model) | ||
| MODEL_NAME="$2" | ||
| shift 2 | ||
| ;; | ||
| --enable-bluetooth) | ||
| ENABLE_BLUETOOTH="yes" | ||
| shift | ||
| ;; | ||
| --disable-bluetooth) | ||
| ENABLE_BLUETOOTH="no" | ||
| shift | ||
| ;; | ||
| --enable-upload) | ||
| ENABLE_UPLOAD="yes" | ||
| shift | ||
| ;; | ||
| --disable-upload) | ||
| ENABLE_UPLOAD="no" | ||
| shift | ||
| ;; | ||
| --upload-port) | ||
| UPLOAD_PORT="$2" | ||
| shift 2 | ||
| ;; |
There was a problem hiding this comment.
📝 Info: Argument parsing with set -u doesn't validate required values
Options like --queue, --device, --model, and --upload-port at scripts/setup_hl_l1222_android_print.sh:114-145 do shift 2 expecting a value argument. If a user passes --queue as the last argument without a value, $2 is unset and set -u (from set -euo pipefail at line 2) will abort with an unhelpful "unbound variable" error rather than a user-friendly message. This is a UX issue rather than a correctness bug — the script still fails safely, just with a confusing error message.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Type=simple | ||
| User=root | ||
| Group=root |
There was a problem hiding this comment.
📝 Info: Hotfolder watcher runs as root while upload service runs as lp
The hotfolder watcher service at scripts/setup_hl_l1222_android_print.sh:445-446 runs as User=root/Group=root, while the upload service at scripts/setup_hl_l1222_android_print.sh:594-595 runs as User=lp/Group=lp. The hotfolder directory is set to 0777 to bridge the permission gap. Running the watcher as root means lp -d print commands execute as root, which works but violates least-privilege. The upload service running as lp is more appropriate. This asymmetry is acknowledged by the security note in the README, but worth noting for hardening.
Was this helpful? React with 👍 or 👎 to provide feedback.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
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:69">
P2: Avoid `--remote-any` here; it unnecessarily opens CUPS access beyond the local subnet for a same-Wi‑Fi AirPrint setup.</violation>
</file>
<file name="scripts/setup_hl_l1222_android_print.sh">
<violation number="1" location="scripts/setup_hl_l1222_android_print.sh:396">
P1: Avoid world-writable hotfolder permissions for a root-processed spool path.</violation>
<violation number="2" location="scripts/setup_hl_l1222_android_print.sh:568">
P2: The optional upload endpoint is unauthenticated while listening on all interfaces.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| exit 1 | ||
| fi | ||
| # Keep folder writable for upload service and watcher workflows. | ||
| if ! sudo chmod 0777 "$HOTFOLDER"; then |
There was a problem hiding this comment.
P1: Avoid world-writable hotfolder permissions for a root-processed spool 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 396:
<comment>Avoid world-writable hotfolder permissions for a root-processed spool path.</comment>
<file context>
@@ -0,0 +1,690 @@
+ exit 1
+ fi
+ # Keep folder writable for upload service and watcher workflows.
+ if ! sudo chmod 0777 "$HOTFOLDER"; then
+ err "Failed to set writable permissions on ${HOTFOLDER}."
+ exit 1
</file context>
|
|
||
| ```bash | ||
| sudo lpadmin -p impressora_brother -o printer-is-shared=true | ||
| sudo cupsctl --share-printers --remote-any |
There was a problem hiding this comment.
P2: Avoid --remote-any here; it unnecessarily opens CUPS access beyond the local subnet for a same-Wi‑Fi AirPrint setup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 69:
<comment>Avoid `--remote-any` here; it unnecessarily opens CUPS access beyond the local subnet for a same-Wi‑Fi AirPrint setup.</comment>
<file context>
@@ -1,3 +1,251 @@
+
+```bash
+sudo lpadmin -p impressora_brother -o printer-is-shared=true
+sudo cupsctl --share-printers --remote-any
+sudo systemctl restart cups
+```
</file context>
| sudo cupsctl --share-printers --remote-any | |
| sudo cupsctl --share-printers |
| self.send_body(200, "<p>Uploaded successfully. File queued.</p><p><a href='/'>Back</a></p>") | ||
|
|
||
| if __name__ == "__main__": | ||
| host = "0.0.0.0" |
There was a problem hiding this comment.
P2: The optional upload endpoint is unauthenticated while listening on all interfaces.
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 568:
<comment>The optional upload endpoint is unauthenticated while listening on all interfaces.</comment>
<file context>
@@ -0,0 +1,690 @@
+ self.send_body(200, "<p>Uploaded successfully. File queued.</p><p><a href='/'>Back</a></p>")
+
+if __name__ == "__main__":
+ host = "0.0.0.0"
+ port = int(os.environ.get("UPLOAD_PORT", "8088"))
+ ThreadingHTTPServer((host, port), H).serve_forever()
</file context>
1 participant
Motivation
Description
scripts/setup_hl_l1222_android_print.sh, a full install/management script that installs dependencies, enables services (cups,avahi-daemon,firewalld), configures CUPS sharing viacupsctl, and creates/repairs a CUPS queue.inotifywaitand a systemd service (hll1222-hotfolder), plus an optional Python upload web service (hll1222-upload) with a systemd unit and configurable port/limits./etc/cups/cupsd.conf, interactive confirmations for risky operations, path resolution helpers, model/URI detection logic, and environment/configuration variables for customization.Testing
Codex Task
Summary by Sourcery
Add a full mobile printing setup for a Raspberry Pi openSUSE CUPS server targeting a USB Brother HL-L1222 printer, including user tutorial and automation scripts.
Enhancements:
Documentation:
Summary by cubic
Adds an iPhone printing tutorial and an automated setup script to turn a Raspberry Pi (openSUSE Tumbleweed) into a CUPS print server for a USB Brother HL‑L1222/HL‑L1222‑V. Enables AirPrint/IPP, a hotfolder auto‑print, and an optional upload fallback for devices that can’t discover the printer.
New Features
scripts/setup_hl_l1222_android_print.shto install and configurecups,avahi-daemon,firewalld; enable sharing viacupsctl; create a CUPS queue; and open firewall forippandmdns.hll1222-hotfolder(usesinotify-tools) and optional upload web servicehll1222-upload(configurable port, usespython3-python-multipart)./etc/cups/cupsd.conf, interactive confirms, URI/model auto‑detect, and simple env‑based config.Migration
scripts/setup_hl_l1222_android_print.shon the Pi; provide--device/--modelonly if auto‑detect fails.Written for commit ff9701f. Summary will update on new commits. Review in cubic