Add setup script and README for Brother HL-L1222-V Android print server (Raspberry Pi / openSUSE)#31
Conversation
Reviewer's GuideAdds a one-shot Bash setup script and accompanying systemd-backed helpers to turn an openSUSE Tumbleweed Raspberry Pi into a shared CUPS IPP print server for a USB Brother HL-L1222-V, plus README documentation for Android/Mopria usage. Sequence diagram for upload-to-hotfolder print workflowsequenceDiagram
actor User
participant Browser as Android_browser
participant Upload as upload_server.py
participant Hotfolder as Hotfolder_directory
participant Watcher as hotfolder_watcher.py
participant CUPS as CUPS_IPP
participant Printer as Brother_HL_L1222_V
User->>Browser: Open http://pi:8088/
Browser->>Upload: GET /
Upload-->>Browser: HTML_upload_form
User->>Browser: Select_file_and_submit_form
Browser->>Upload: POST /upload (multipart_file)
Upload->>Upload: Validate_size_and_extension
Upload->>Hotfolder: Write_file_to_hotfolder
Upload-->>Browser: 200_OK_uploaded_successfully
loop Periodic_scan
Watcher->>Hotfolder: List_and_check_files
Watcher->>Watcher: Wait_for_stable_file_size
Watcher->>CUPS: lp -d Brother_HL_L1222_V file
CUPS->>Printer: Send_print_job_via_USB
Watcher->>Hotfolder: Move_file_to_printed_or_failed
end
Flow diagram for setup_hl_l1222_android_print.sh executiongraph TD
A[start_script]
A --> B[parse_args_and_flags]
B --> C[resolve_and_require_sudo_zypper_lpinfo_lpadmin_lpstat]
C --> D{--status_flag_set?}
D -- yes --> S[status_report_and_exit]
D -- no --> E[install_dependencies_via_zypper]
E --> F[enable_and_start_cups_avahi_firewalld_optional_bluetooth]
F --> G[configure_firewalld_for_ipp_mdns_optional_upload_port]
G --> H[backup_and_configure_cupsd.conf_for_sharing_and_mDNS]
H --> I[detect_usb_device_uri_for_Brother_printer]
I --> J[select_best_model_brlaser_else_everywhere_else_generic]
J --> K{queue_already_exists?}
K -- yes --> K1[ask_user_confirmation_and_delete_existing_queue]
K1 --> L[create_new_queue_with_lpadmin_and_set_defaults]
K -- no --> L
L --> M[enable_accept_queue_and_set_default_printer]
M --> N[restart_cups]
N --> O[install_hotfolder_watcher_script_and_systemd_service]
O --> P{upload_enabled?}
P -- yes --> Q[install_upload_server_script_and_systemd_service]
P -- no --> R[skip_upload_service_installation]
Q --> T[print_summary_with_IPP_URL_and_optional_upload_URL]
R --> T
T --> U[end]
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 comprehensive setup script and documentation for configuring a Raspberry Pi as a print server for the Brother HL-L1222-V printer on openSUSE Tumbleweed. The script automates dependency installation, CUPS sharing configuration, and the deployment of a Python-based hotfolder service for automated printing. Review feedback focuses on improving the robustness of the CUPS configuration by utilizing cupsctl for all directives, optimizing the file stability check in the watcher service to prevent blocking, and ensuring consistent binary path resolution for all system utilities.
| if ! sudo cupsctl --share-printers --remote-any; then | ||
| err "Failed to configure cups sharing with cupsctl." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Ensure WebInterface and mDNS browsing directives exist. | ||
| if ! sudo grep -qi '^WebInterface[[:space:]]\+Yes' "${CUPSD_CONF}"; then | ||
| echo 'WebInterface Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null | ||
| fi | ||
| if ! sudo grep -qi '^Browsing[[:space:]]\+Yes' "${CUPSD_CONF}"; then | ||
| echo 'Browsing Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null | ||
| fi | ||
| if ! sudo grep -qi '^BrowseLocalProtocols[[:space:]]\+dnssd' "${CUPSD_CONF}"; then | ||
| echo 'BrowseLocalProtocols dnssd' | sudo tee -a "${CUPSD_CONF}" >/dev/null | ||
| fi |
There was a problem hiding this comment.
Using cupsctl to set all configuration directives at once is more robust and cleaner than manually appending to cupsd.conf. cupsctl handles existing entries correctly and ensures proper syntax, avoiding potential duplicate or conflicting directives in the configuration file.
| if ! sudo cupsctl --share-printers --remote-any; then | |
| err "Failed to configure cups sharing with cupsctl." | |
| exit 1 | |
| fi | |
| # Ensure WebInterface and mDNS browsing directives exist. | |
| if ! sudo grep -qi '^WebInterface[[:space:]]\+Yes' "${CUPSD_CONF}"; then | |
| echo 'WebInterface Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null | |
| fi | |
| if ! sudo grep -qi '^Browsing[[:space:]]\+Yes' "${CUPSD_CONF}"; then | |
| echo 'Browsing Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null | |
| fi | |
| if ! sudo grep -qi '^BrowseLocalProtocols[[:space:]]\+dnssd' "${CUPSD_CONF}"; then | |
| echo 'BrowseLocalProtocols dnssd' | sudo tee -a "${CUPSD_CONF}" >/dev/null | |
| fi | |
| log "Configuring CUPS sharing using cupsctl..." | |
| if ! sudo "$CUPSCTL_BIN" --share-printers --remote-any WebInterface=Yes Browsing=Yes BrowseLocalProtocols="dnssd"; then | |
| err "Failed to configure cups sharing with cupsctl." | |
| exit 1 | |
| fi |
| exit 1 | ||
| fi | ||
|
|
||
| if ! sudo "$LPADMIN_BIN" -p "$QUEUE_NAME" -o printer-is-shared=true -o media=A4 -o PageSize=A4 -o sides=one-sided; then |
There was a problem hiding this comment.
The paper size is hardcoded to A4. This may cause printing issues (e.g., "wrong paper size" errors) for users in regions where Letter is the standard. It is recommended to add a comment or make this configurable to improve usability across different regions.
| if ! sudo "$LPADMIN_BIN" -p "$QUEUE_NAME" -o printer-is-shared=true -o media=A4 -o PageSize=A4 -o sides=one-sided; then | |
| # Set queue defaults (Note: media is set to A4; change to 'Letter' if required) | |
| if ! sudo "$LPADMIN_BIN" -p "$QUEUE_NAME" -o printer-is-shared=true -o media=A4 -o PageSize=A4 -o sides=one-sided; then |
| def stable(p: Path) -> bool: | ||
| try: | ||
| s1 = p.stat().st_size | ||
| time.sleep(1) | ||
| s2 = p.stat().st_size | ||
| return s1 == s2 and s2 > 0 | ||
| except FileNotFoundError: | ||
| return False |
There was a problem hiding this comment.
The current stable function uses time.sleep(1) inside a loop that iterates over files. This causes a linear delay for every new file in the hotfolder, blocking the entire watcher process. A more efficient approach is to check if the file's modification time is old enough (e.g., > 2 seconds), which indicates the write operation has likely finished without blocking the loop execution.
| def stable(p: Path) -> bool: | |
| try: | |
| s1 = p.stat().st_size | |
| time.sleep(1) | |
| s2 = p.stat().st_size | |
| return s1 == s2 and s2 > 0 | |
| except FileNotFoundError: | |
| return False | |
| def stable(p: Path) -> bool: | |
| try: | |
| st = p.stat() | |
| # File is stable if it hasn't been modified for at least 2 seconds and has content | |
| return st.st_size > 0 and (time.time() - st.st_mtime) > 2 | |
| except FileNotFoundError: | |
| return False |
| require_command "lpinfo" "cups-client" | ||
| require_command "lpadmin" "cups-client" | ||
| require_command "lpstat" "cups-client" | ||
|
|
||
| LPINFO_BIN="$(resolve_bin lpinfo || true)" | ||
| LPADMIN_BIN="$(resolve_bin lpadmin || true)" | ||
| LPSTAT_BIN="$(resolve_bin lpstat || true)" | ||
| if [[ -z "$LPINFO_BIN" || -z "$LPADMIN_BIN" || -z "$LPSTAT_BIN" ]]; then | ||
| err "lpinfo/lpadmin/lpstat not resolvable in PATH or standard sbin/bin directories." | ||
| err "Try: sudo zypper install cups-client && reopen shell, then rerun." | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
For consistency and robustness, cupsctl should be checked and resolved to its full path like lpadmin and lpinfo. This ensures the script uses the correct binary and provides a clear error message if it's missing.
| require_command "lpinfo" "cups-client" | |
| require_command "lpadmin" "cups-client" | |
| require_command "lpstat" "cups-client" | |
| LPINFO_BIN="$(resolve_bin lpinfo || true)" | |
| LPADMIN_BIN="$(resolve_bin lpadmin || true)" | |
| LPSTAT_BIN="$(resolve_bin lpstat || true)" | |
| if [[ -z "$LPINFO_BIN" || -z "$LPADMIN_BIN" || -z "$LPSTAT_BIN" ]]; then | |
| err "lpinfo/lpadmin/lpstat not resolvable in PATH or standard sbin/bin directories." | |
| err "Try: sudo zypper install cups-client && reopen shell, then rerun." | |
| exit 1 | |
| fi | |
| require_command "lpinfo" "cups-client" | |
| require_command "lpadmin" "cups-client" | |
| require_command "lpstat" "cups-client" | |
| require_command "cupsctl" "cups" | |
| LPINFO_BIN="$(resolve_bin lpinfo || true)" | |
| LPADMIN_BIN="$(resolve_bin lpadmin || true)" | |
| LPSTAT_BIN="$(resolve_bin lpstat || true)" | |
| CUPSCTL_BIN="$(resolve_bin cupsctl || true)" | |
| if [[ -z "$LPINFO_BIN" || -z "$LPADMIN_BIN" || -z "$LPSTAT_BIN" || -z "$CUPSCTL_BIN" ]]; then | |
| err "lpinfo/lpadmin/lpstat/cupsctl not resolvable in PATH or standard sbin/bin directories." | |
| err "Try: sudo zypper install cups cups-client && reopen shell, then rerun." | |
| exit 1 | |
| fi |
![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: b5072d2a47
ℹ️ 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 "$LPINFO_BIN" -m | awk '{print $1}' | grep -qx 'everywhere'; then | ||
| MODEL_NAME="everywhere" | ||
| warn "Falling back to 'everywhere' model." |
There was a problem hiding this comment.
everywhere for directly attached USB URIs
When brlaser is unavailable, the fallback at this branch sets MODEL_NAME="everywhere", but this script always builds a local USB queue (direct usb://...) in detect_usb_uri. Per lpadmin(8), -m everywhere is intended to query an IPP device URI, so this fallback can cause queue creation to fail on exactly the recovery path meant to keep setup working (e.g., no Brother PPD available).
Useful? React with 👍 / 👎.
![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:
- The
--statusflag likely never takes effect becauseSTATUS_ONLYis declaredlocalinsidemainwhileparse_argswrites to the global variable; remove thelocalor pass the flag via return value/explicit parameter instead of relying on a global. - In
print_summary, the lineecho " "$LPINFO_BIN" -m | grep -Ei ...breaks quoting and will not print the intended command; wrap the whole command in a single set of quotes or escape the$so it prints literally (e.g.,echo " $LPINFO_BIN -m | grep -Ei '...'").
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `--status` flag likely never takes effect because `STATUS_ONLY` is declared `local` inside `main` while `parse_args` writes to the global variable; remove the `local` or pass the flag via return value/explicit parameter instead of relying on a global.
- In `print_summary`, the line `echo " "$LPINFO_BIN" -m | grep -Ei ...` breaks quoting and will not print the intended command; wrap the whole command in a single set of quotes or escape the `$` so it prints literally (e.g., `echo " $LPINFO_BIN -m | grep -Ei '...'"`).
## Individual Comments
### Comment 1
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="677-686" />
<code_context>
+}
+
+main() {
+ local STATUS_ONLY="no"
+ parse_args "$@"
+
+ require_command "sudo" "sudo"
+ require_command "zypper" "zypper"
+ require_command "lpinfo" "cups-client"
+ require_command "lpadmin" "cups-client"
+ require_command "lpstat" "cups-client"
+
+ LPINFO_BIN="$(resolve_bin lpinfo || true)"
+ LPADMIN_BIN="$(resolve_bin lpadmin || true)"
+ LPSTAT_BIN="$(resolve_bin lpstat || true)"
+ if [[ -z "$LPINFO_BIN" || -z "$LPADMIN_BIN" || -z "$LPSTAT_BIN" ]]; then
+ err "lpinfo/lpadmin/lpstat not resolvable in PATH or standard sbin/bin directories."
+ err "Try: sudo zypper install cups-client && reopen shell, then rerun."
+ exit 1
+ fi
+
+ if [[ "${STATUS_ONLY:-no}" == "yes" ]]; then
+ status_report
+ exit 0
</code_context>
<issue_to_address>
**issue (bug_risk):** The --status flag never takes effect because STATUS_ONLY is local to main while parse_args writes a separate global variable.
Because `STATUS_ONLY` is declared `local` in `main`, `parse_args` only updates the global `STATUS_ONLY`, while the condition in `main` still reads the unchanged local value. Consider either making `STATUS_ONLY` global (remove `local` in `main`) or having `parse_args` return or set a different variable that `main` then uses to set its local `STATUS_ONLY`.
</issue_to_address>
### Comment 2
<location path="scripts/setup_hl_l1222_android_print.sh" line_range="662" />
<code_context>
+ echo "Validate:"
+ echo " $LPSTAT_BIN -t"
+ echo " lpinfo -v"
+ echo " "$LPINFO_BIN" -m | grep -Ei 'brother|hl|brlaser|everywhere|generic|laser|mono|pcl'"
+ echo " echo 'test page from pi' | lp -d ${QUEUE_NAME}"
+ echo
</code_context>
<issue_to_address>
**issue (bug_risk):** The quoting here makes the grep command execute instead of printing the literal command string.
Because the quotes are broken around `$LPINFO_BIN`, the shell interprets this as `echo /path/to/lpinfo -m | grep -Ei ...`, so `grep` actually runs instead of showing a copy/pasteable command. If you intend to display the command, wrap the whole pipeline in a single quoted string and interpolate `${LPINFO_BIN}`, for example:
```bash
echo " ${LPINFO_BIN} -m | grep -Ei 'brother|hl|brlaser|everywhere|generic|laser|mono|pcl'"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| local STATUS_ONLY="no" | ||
| parse_args "$@" | ||
|
|
||
| require_command "sudo" "sudo" | ||
| require_command "zypper" "zypper" | ||
| require_command "lpinfo" "cups-client" | ||
| require_command "lpadmin" "cups-client" | ||
| require_command "lpstat" "cups-client" | ||
|
|
||
| LPINFO_BIN="$(resolve_bin lpinfo || true)" |
There was a problem hiding this comment.
issue (bug_risk): The --status flag never takes effect because STATUS_ONLY is local to main while parse_args writes a separate global variable.
Because STATUS_ONLY is declared local in main, parse_args only updates the global STATUS_ONLY, while the condition in main still reads the unchanged local value. Consider either making STATUS_ONLY global (remove local in main) or having parse_args return or set a different variable that main then uses to set its local STATUS_ONLY.
| echo "Validate:" | ||
| echo " $LPSTAT_BIN -t" | ||
| echo " lpinfo -v" | ||
| echo " "$LPINFO_BIN" -m | grep -Ei 'brother|hl|brlaser|everywhere|generic|laser|mono|pcl'" |
There was a problem hiding this comment.
issue (bug_risk): The quoting here makes the grep command execute instead of printing the literal command string.
Because the quotes are broken around $LPINFO_BIN, the shell interprets this as echo /path/to/lpinfo -m | grep -Ei ..., so grep actually runs instead of showing a copy/pasteable command. If you intend to display the command, wrap the whole pipeline in a single quoted string and interpolate ${LPINFO_BIN}, for example:
echo " ${LPINFO_BIN} -m | grep -Ei 'brother|hl|brlaser|everywhere|generic|laser|mono|pcl'"![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| exit 1 | ||
| fi | ||
|
|
||
| TMP_DIR="" |
There was a problem hiding this comment.
🟡 Temp directories leaked: TMP_DIR="" clears tracking without removing the directory
In install_hotfolder_service (line 494) and install_upload_service_optional (line 643), TMP_DIR is set to "" after the temp directory's contents have been installed. However, the actual temp directory created by mktemp -d (scripts/setup_hl_l1222_android_print.sh:396 and scripts/setup_hl_l1222_android_print.sh:510) is never removed. The cleanup trap at scripts/setup_hl_l1222_android_print.sh:62-66 only acts when TMP_DIR is non-empty, so on successful script completion both temp directories remain as orphaned directories in /tmp.
| TMP_DIR="" | |
| rm -rf "$TMP_DIR" | |
| TMP_DIR="" |
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.
🟡 Upload filename truncation to 160 chars can silently drop file extension, causing hotfolder rejection
In the embedded upload server's do_POST, the sanitized filename is truncated to 160 characters at line 572: safe = "...".join(...)[:160]. If the original filename exceeds 160 chars, the extension (e.g. .pdf) is lost from safe. The file is then saved to disk without its extension. The hotfolder watcher (scripts/setup_hl_l1222_android_print.sh:433) checks f.suffix.lower() not in allowed and moves the extension-less file to failed/, even though the upload server reported success to the user at line 585. Note the inconsistency: the collision-resolution path at line 576 appends the original ext, but the initial non-collision path at line 573 uses safe directly with the truncated (extension-less) name.
| safe = "".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in name)[:160] | |
| dst = hot / safe | |
| safe = ".".join(ch if ch.isalnum() or ch in "._-" else "_" for ch in Path(name).stem)[:160 - len(ext)] + ext | |
| dst = hot / safe |
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="" at line 643 clears the tracking variable without removing the temp directory created at scripts/setup_hl_l1222_android_print.sh:510. This is the second instance of the same pattern — the actual rm -rf is missing before clearing the variable.
| TMP_DIR="" | |
| rm -rf "$TMP_DIR" | |
| TMP_DIR="" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| # Execution context: | ||
| # Run as a normal user from home directory (~/). Script uses sudo for | ||
| # privileged operations. |
There was a problem hiding this comment.
🚩 Script execution context contradicts README instructions
The script's header comments at lines 15-16 say "Run as a normal user from home directory (~/)" and "Script uses sudo for privileged operations." However, the README instructs sudo bash scripts/setup_hl_l1222_android_print.sh (running the entire script as root). More importantly, several lpinfo calls (scripts/setup_hl_l1222_android_print.sh:294, scripts/setup_hl_l1222_android_print.sh:314, scripts/setup_hl_l1222_android_print.sh:321, scripts/setup_hl_l1222_android_print.sh:328) are invoked without sudo, while lpadmin calls at lines 346, 353, 358 use sudo. Since lpinfo typically requires root/lpadmin group, the script would fail if run as a normal user as the comments suggest. The README's sudo bash approach works, but the inconsistency is confusing and the script's own documentation is misleading.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if not CGI_AVAILABLE: | ||
| self.send_body(503, "Upload disabled: Python cgi module not available (Python 3.13+).") | ||
| return | ||
| clen = int(self.headers.get("Content-Length", "0")) |
There was a problem hiding this comment.
📝 Info: Upload server int(Content-Length) can raise ValueError on malformed requests
At scripts/setup_hl_l1222_android_print.sh:558, clen = int(self.headers.get("Content-Length", "0")) will raise ValueError if a client sends a non-numeric Content-Length header (e.g., Content-Length: abc). This was considered as a bug but dismissed because ThreadingHTTPServer handles each request in a separate thread, and BaseServer._handle_request_noblock catches unhandled exceptions via handle_error(), so the server continues running. The client receives a dropped connection rather than a proper 4xx error, which is suboptimal but not a crash.
Was this helpful? React with 👍 or 👎 to provide feedback.
| i = 1 | ||
| while dst.exists(): | ||
| dst = hot / f"{Path(safe).stem}_{i}{ext}" | ||
| i += 1 |
There was a problem hiding this comment.
📝 Info: Upload server TOCTOU race in filename collision resolution
At lines 574-577, the upload server checks dst.exists() and increments a counter to find a unique filename. With ThreadingHTTPServer, two concurrent uploads with the same filename could both pass the existence check simultaneously and write to the same path, causing one upload to silently overwrite the other. This was not reported as a bug because the upload service is optional, the race window is narrow, and concurrent uploads of identically-named files to a single-printer hotfolder is an unlikely scenario. However, if robustness is desired, using os.open with O_CREAT | O_EXCL would eliminate the race.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
|
||
| while True: | ||
| for f in sorted(hot.iterdir()): | ||
| if f.is_dir() or f.name.startswith('.'): |
There was a problem hiding this comment.
📝 Info: Upload server dot-prefixed filenames silently accepted but never printed
If a user uploads a file whose sanitized name starts with . (e.g., .config.pdf), the upload server accepts it and reports success (scripts/setup_hl_l1222_android_print.sh:585), but the hotfolder watcher skips files starting with . (scripts/setup_hl_l1222_android_print.sh:430). The file would sit in the hotfolder indefinitely, never printed and never moved to failed/. This is an edge case since most real uploads won't have dot-prefixed names, but it creates a misleading user experience when it does occur.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if ! sudo grep -qi '^WebInterface[[:space:]]\+Yes' "${CUPSD_CONF}"; then | ||
| echo 'WebInterface Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null | ||
| fi | ||
| if ! sudo grep -qi '^Browsing[[:space:]]\+Yes' "${CUPSD_CONF}"; then | ||
| echo 'Browsing Yes' | sudo tee -a "${CUPSD_CONF}" >/dev/null | ||
| fi | ||
| if ! sudo grep -qi '^BrowseLocalProtocols[[:space:]]\+dnssd' "${CUPSD_CONF}"; then | ||
| echo 'BrowseLocalProtocols dnssd' | sudo tee -a "${CUPSD_CONF}" >/dev/null | ||
| fi |
There was a problem hiding this comment.
📝 Info: CUPS config grep patterns may cause duplicate directives
At lines 272-279, the script checks for existing CUPS directives using grep -qi before appending. However, cupsctl --share-printers --remote-any (line 266) may set some of these directives with different formatting (e.g., different whitespace). The grep patterns use \+ (one-or-more spaces) which works with GNU grep's BRE, but if cupsctl writes e.g. BrowseLocalProtocols dnssd with two spaces, or the directive already exists with a different value like BrowseLocalProtocols dnssd cups, the grep could match or miss. If there's a mismatch, duplicate/conflicting directives could be appended. On the target platform (openSUSE with GNU grep), this is unlikely but worth noting.
Was this helpful? React with 👍 or 👎 to provide feedback.
| echo "Validate:" | ||
| echo " $LPSTAT_BIN -t" | ||
| echo " lpinfo -v" | ||
| echo " "$LPINFO_BIN" -m | grep -Ei 'brother|hl|brlaser|everywhere|generic|laser|mono|pcl'" |
There was a problem hiding this comment.
📝 Info: Line 662 has unusual quoting style but works correctly by accident
The echo statement echo " "$LPINFO_BIN" -m | grep ..." at line 662 has the variable $LPINFO_BIN sitting between two double-quoted segments. This is technically unquoted variable expansion, but because the adjacent quoted segments form a single word and the variable contains a simple path without spaces, the output is correct. The intended form is likely echo " $LPINFO_BIN -m | grep ..." with the variable inside a single set of double quotes. Not flagged as a bug since the behavior is identical for standard system paths.
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.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="README.md">
<violation number="1" location="README.md:29">
P3: This line documents model selection incorrectly: the script does not default to `everywhere`; it prefers `brlaser`/Brother models and only falls back to `everywhere`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| - `ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V` | ||
|
|
||
| ## Notes | ||
| - Script uses CUPS `everywhere` model as default for broad compatibility. |
There was a problem hiding this comment.
P3: This line documents model selection incorrectly: the script does not default to everywhere; it prefers brlaser/Brother models and only falls back to everywhere.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 29:
<comment>This line documents model selection incorrectly: the script does not default to `everywhere`; it prefers `brlaser`/Brother models and only falls back to `everywhere`.</comment>
<file context>
@@ -1,3 +1,30 @@
+ - `ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V`
+
+## Notes
+- Script uses CUPS `everywhere` model as default for broad compatibility.
+- If you have an official Brother HL-L1222-V Linux PPD/driver package, install it and adjust queue model if desired.
</file context>
| - Script uses CUPS `everywhere` model as default for broad compatibility. | |
| - Script prefers `brlaser`/Brother models when available, then falls back to CUPS `everywhere` for broad compatibility. |
1 participant
Motivation
Description
scripts/setup_hl_l1222_android_print.sh, a robust Bash script that installs dependencies, enablescups,avahi,firewalld, optional Bluetooth, configures CUPS sharing, opens firewall rules, detects the Brother USB device, picks a suitable PPD/model (prefersbrlaser, falls back toeverywhereor a generic mono laser), and creates/replaces the CUPS queue withlpadminand related commands.hotfolder_watcher.pyand a systemd unithll1222-hotfolder.serviceto auto-print files placed into/var/spool/hll1222-hotfolder.upload_server.pyand systemd unithll1222-upload.service(enabled when--enable-uploadis passed), with safeguards for Python 3.13+ where thecgimodule may be missing.README.mdwith purpose, usage (sudo bash scripts/setup_hl_l1222_android_print.sh), post-setup instructions, connection examples (IPP URL), and notes about Bluetooth and model selection.Testing
Codex Task
Summary by Sourcery
Add an automated setup for configuring a Raspberry Pi (openSUSE Tumbleweed) as a CUPS/IPPs print server for a USB-connected Brother HL-L1222/HL-L1222-V printer, including optional hotfolder and upload-based printing workflows for Android devices.
New Features:
Enhancements:
Documentation:
Summary by cubic
Adds a one-step setup to turn a Raspberry Pi (openSUSE Tumbleweed) into a CUPS/IPP print server for a USB Brother HL‑L1222/HL‑L1222‑V so Android devices can print over Wi‑Fi. Includes an auto-print hotfolder and an optional LAN upload service.
New Features
scripts/setup_hl_l1222_android_print.shinstalls and configurescups,avahi,firewalld(opensippandmdns), and optional Bluetooth.Brother_HL_L1222_Vusinglpadmin; prefersbrlaser, falls back toeverywhereor a generic mono model.cupsd.confand interactive confirmations for risky changes.hotfolder_watcher.py) withhll1222-hotfolder.serviceto auto-print files from/var/spool/hll1222-hotfolder.upload_server.py,hll1222-upload.service) behind a size cap; skipped automatically on Python 3.13+ ifcgiis unavailable.Migration
sudo bash scripts/setup_hl_l1222_android_print.sh(use--enable-uploadto enable the upload page).ipp://<raspberrypi-ip>/printers/Brother_HL_L1222_V.http://<raspberrypi-ip>:8088/.Written for commit b5072d2. Summary will update on new commits. Review in cubic