Adds support for printer configuration #422
Conversation
|
Hi @frenck just wondering if everything is OK with this PR seeing as you have merged others... Don't hesitate to let me know if you spot any concerns, etc. and thanks for this great addon. |
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
github-actions
bot
added
the
stale
|
Well, can we unstale this, just nobody reviewed or answered... |
github-actions
bot
removed
the
stale
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
github-actions
bot
added
the
stale
frenck
added
no-stale
Adds support for printer configuration
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- grocy/DOCS.md (2 hunks)
- grocy/config.yaml (3 hunks)
- grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (3 hunks)
Additional Context Used
LanguageTool (16)
grocy/DOCS.md (16)
Near line 74: Loose punctuation mark.
Context: ...n issue. Possible values are: -trace: Show every detail, like all called inte...
Near line 75: Loose punctuation mark.
Context: ...all called internal functions. -debug: Shows detailed debug information. - `in...
Near line 76: Loose punctuation mark.
Context: ...ows detailed debug information. -info: Normal (usually) interesting events. - ...
Near line 77: Loose punctuation mark.
Context: ...usually) interesting events. -warning: Exceptional occurrences that are not er...
Near line 78: Loose punctuation mark.
Context: ...currences that are not errors. -error: Runtime errors that do not require imme...
Near line 79: Loose punctuation mark.
Context: ... not require immediate action. -fatal: Something went terribly wrong. Add-on b...
Near line 104: This sentence seems to be incomplete. Insert a noun before ‘Is’ to make the sentence complete.
Context: ...is the default_ ### Option:cultureIs used for setting the language. Choose b...
Near line 144: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...on:entry_pageAllows you to specify an custom homepage if desired. You can us...
Near line 146: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ustom homepage if desired. You can use the one of the following values: -batteries- ...
Near line 157: Did you mean: “By default,”?
Context: ...-shoppinglist-stock-tasks` By default the homepage is set to the stock overvi...
Near line 161: Possible missing preposition found.
Context: ...tock overview. ### Option:featuresIs used for enable or disable features in ...
Near line 162: The preposition ‘to’ seems more likely in this position.
Context: ...rview. ### Option:featuresIs used for enable or disable features in Grocy. Di...
Near line 226: Consider using “who” when you are referring to a person instead of an object.
Context: ...to '/dev/ttyS0' Make sure that the user that runs the webserver has permissions to w...
Near line 243: Loose punctuation mark.
Context: ...ented based on the following: -MAJOR: Incompatible or major changes. - `MINOR...
Near line 244: Loose punctuation mark.
Context: ...Incompatible or major changes. -MINOR: Backwards-compatible new features and e...
Near line 245: Loose punctuation mark.
Context: ...new features and enhancements. -PATCH: Backwards-compatible bugfixes and packa...
Additional comments not posted (6)
grocy/config.yaml (3)
36-37: The addition oflabel_printerandthermal_printeroptions under thefeaturessection is well-implemented. This allows users to enable or disable these features as needed.
65-66: The schema definitions forlabel_printerandthermal_printeras booleans are correctly added. This ensures that the configuration values for these features are validated as boolean types.
78-87: The schema additions for the new printer settings are comprehensive and well-detailed. However, ensure that the range fortprinter_portis appropriate and that the data types are consistent with the expected inputs.grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (2)
9-13: The declaration of variables related to printer configurations (label_printer_webhook,label_printer_params,tprinter_ip,tprinter_port,tprinter_connector) is correctly implemented. This prepares the script to handle these configurations effectively.
52-58: The conditional export of feature flags for label and thermal printers based on their configuration is correctly implemented. This ensures that the feature flags are only set when the features are enabled.grocy/DOCS.md (1)
174-175: The documentation update to includelabel_printerandthermal_printerunder thefeaturessection is clear and informative. It correctly guides the user on how to enable or disable these features.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded label and thermal printer support: docs for printing, new feature toggles and tweak keys in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Config as config.yaml
participant Init as s6 php-fpm run
participant Env as Environment
participant App as Grocy PHP-FPM
participant Printer as Label/Thermal Printer
Note over Config,Init: Startup reads features & tweaks
Config->>Init: feature toggles + tweak values (label_printer, thermal_printer, label_* , tprinter_*)
Init->>Init: evaluate flags, set local vars, sanitize values
Init->>Env: conditional export of GROCY_LABEL_PRINTER_* and GROCY_TPRINTER_*
Env->>App: PHP-FPM inherits env
alt label_printer enabled
App->>Printer: invoke label webhook (params or JSON)
end
alt thermal_printer enabled
App->>Printer: send ESC/POS job (network IP:PORT or local connector)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
grocy/DOCS.md (4)
Line range hint
144-146: Clarify the language for setting a custom homepage.- Allows you to specify an custom homepage if desired. + Allows you to specify a custom homepage if desired. - You can use the one of the following values: + You can use one of the following values:
Line range hint
157-157: Clarify default setting language.- By default the homepage is set to the stock overview. + By default, the homepage is set to the stock overview.
Line range hint
161-161: Improve grammar in feature description.- Is used for enable or disable features in Grocy. + Is used to enable or disable features in Grocy.
Line range hint
104-104: Clarify language setting description.- Is used for setting the language. + Is used to set the language.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- grocy/DOCS.md (2 hunks)
- grocy/config.yaml (3 hunks)
- grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- grocy/config.yaml
- grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run
Additional Context Used
LanguageTool (15)
grocy/DOCS.md (15)
Near line 74: Loose punctuation mark.
Context: ...n issue. Possible values are: -trace: Show every detail, like all called inte...
Near line 75: Loose punctuation mark.
Context: ...all called internal functions. -debug: Shows detailed debug information. - `in...
Near line 76: Loose punctuation mark.
Context: ...ows detailed debug information. -info: Normal (usually) interesting events. - ...
Near line 77: Loose punctuation mark.
Context: ...usually) interesting events. -warning: Exceptional occurrences that are not er...
Near line 78: Loose punctuation mark.
Context: ...currences that are not errors. -error: Runtime errors that do not require imme...
Near line 79: Loose punctuation mark.
Context: ... not require immediate action. -fatal: Something went terribly wrong. Add-on b...
Near line 104: This sentence seems to be incomplete. Insert a noun before ‘Is’ to make the sentence complete.
Context: ...is the default_ ### Option:cultureIs used for setting the language. Choose b...
Near line 144: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...on:entry_pageAllows you to specify an custom homepage if desired. You can us...
Near line 146: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ustom homepage if desired. You can use the one of the following values: -batteries- ...
Near line 157: Did you mean: “By default,”?
Context: ...-shoppinglist-stock-tasks` By default the homepage is set to the stock overvi...
Near line 161: This sentence seems to be incomplete. Insert a noun before ‘Is’ to make the sentence complete.
Context: ...tock overview. ### Option:featuresIs used for enable or disable features in ...
Near line 226: Consider using “who” when you are referring to a person instead of an object.
Context: ...to '/dev/ttyS0' Make sure that the user that runs the webserver has permissions to w...
Near line 243: Loose punctuation mark.
Context: ...ented based on the following: -MAJOR: Incompatible or major changes. - `MINOR...
Near line 244: Loose punctuation mark.
Context: ...Incompatible or major changes. -MINOR: Backwards-compatible new features and e...
Near line 245: Loose punctuation mark.
Context: ...new features and enhancements. -PATCH: Backwards-compatible bugfixes and packa...
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (5)
grocy/DOCS.md (5)
174-175: Add a brief description forlabel_printerandthermal_printeroptions.It would enhance clarity to include a brief description of what these options do, similar to other feature descriptions in the document.
Line range hint
144-146: Correct grammatical errors in the documentation.- Allows you to specify an custom homepage if desired. + Allows you to specify a custom homepage if desired. - You can use the one of the following values: + You can use one of the following values:
Line range hint
157-157: Clarify the default setting description.- By default the homepage is set to the stock overview. + By default, the homepage is set to the stock overview.
Line range hint
161-161: Improve the sentence structure for feature options.- Is used for enable or disable features in Grocy. + This option is used to enable or disable features in Grocy.
Line range hint
104-104: Clarify the language option description.- Is used for setting the language. + This option is used for setting the language.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- grocy/DOCS.md (2 hunks)
Additional Context Used
LanguageTool (15)
grocy/DOCS.md (15)
Near line 74: Loose punctuation mark.
Context: ...n issue. Possible values are: -trace: Show every detail, like all called inte...
Near line 75: Loose punctuation mark.
Context: ...all called internal functions. -debug: Shows detailed debug information. - `in...
Near line 76: Loose punctuation mark.
Context: ...ows detailed debug information. -info: Normal (usually) interesting events. - ...
Near line 77: Loose punctuation mark.
Context: ...usually) interesting events. -warning: Exceptional occurrences that are not er...
Near line 78: Loose punctuation mark.
Context: ...currences that are not errors. -error: Runtime errors that do not require imme...
Near line 79: Loose punctuation mark.
Context: ... not require immediate action. -fatal: Something went terribly wrong. Add-on b...
Near line 104: This sentence seems to be incomplete. Insert a noun before ‘Is’ to make the sentence complete.
Context: ...is the default_ ### Option:cultureIs used for setting the language. Choose b...
Near line 144: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...on:entry_pageAllows you to specify an custom homepage if desired. You can us...
Near line 146: Make sure that ‘the one of’ is correct and that ‘one’ is a pronoun. Possibly, the ‘the’ is unnecessary or ‘of’ is better expressed with a preposition such as ‘about’ or ‘in’.
Context: ...ustom homepage if desired. You can use the one of the following values: -batteries- ...
Near line 157: Did you mean: “By default,”?
Context: ...-shoppinglist-stock-tasks` By default the homepage is set to the stock overvi...
Near line 161: This sentence seems to be incomplete. Insert a noun before ‘Is’ to make the sentence complete.
Context: ...tock overview. ### Option:featuresIs used for enable or disable features in ...
Near line 226: Consider using “who” when you are referring to a person instead of an object.
Context: ...to '/dev/ttyS0' Make sure that the user that runs the webserver has permissions to w...
Near line 242: Loose punctuation mark.
Context: ...ented based on the following: -MAJOR: Incompatible or major changes. - `MINOR...
Near line 243: Loose punctuation mark.
Context: ...Incompatible or major changes. -MINOR: Backwards-compatible new features and e...
Near line 244: Loose punctuation mark.
Context: ...new features and enhancements. -PATCH: Backwards-compatible bugfixes and packa...
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Is there a problem blocking this PR from being merged? I bought a new label printer for Grocy and really want to use it. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (2)
9-13: Consider prefixing all environment variables consistently.Currently, label printer environment variables are prefixed sometimes with “LABEL_PRINTER_” (e.g., LABEL_PRINTER_WEBHOOK) and sometimes with “GROCY_LABEL_PRINTER_” (e.g., GROCY_LABEL_PRINTER_RUN_SERVER). Similarly, the thermal printer feature flag is missing the “GROCY_” prefix. This mismatch may cause confusion.
52-59: Align naming conventions of feature flags.The label printer feature flag is exported as “GROCY_FEATURE_FLAG_LABEL_PRINTER,” while the thermal printer feature flag is exported as “FEATURE_FLAG_THERMAL_PRINTER.” For consistency, consider using the same prefix.
grocy/DOCS.md (1)
218-226: Minor grammar improvement suggestion: use “who” instead of “that.”In “the user that runs the webserver,” consider using “who” for better clarity.
Proposed edit:
- Make sure that the user that runs the webserver has permissions + Make sure that the user who runs the webserver has permissions🧰 Tools
🪛 LanguageTool
[style] ~226-~226: Consider using “who” when you are referring to a person instead of an object.
Context: ...to '/dev/ttyS0' Make sure that the user that runs the webserver has permissions to w...(THAT_WHO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
grocy/DOCS.md(2 hunks)grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run(3 hunks)
🧰 Additional context used
🪛 LanguageTool
grocy/DOCS.md
[style] ~226-~226: Consider using “who” when you are referring to a person instead of an object.
Context: ...to '/dev/ttyS0' Make sure that the user that runs the webserver has permissions to w...
(THAT_WHO)
🔇 Additional comments (5)
grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (3)
102-105: Good addition for label printer integration.
This logic dynamically sets and exports the webhook environment variable for label printing. Straightforward and clear.
120-145: Thermal printer environment variables look consistent.
The conditionals to set different parameters (IP, port, connector) are clean and straightforward. Nicely done!
116-118: Verify JSON vs. form-data usage.
Ensure that external code or scripts checking this variable handle the difference between JSON and form-data correctly, to avoid unexpected parsing errors on the receiving side.
grocy/DOCS.md (2)
174-175: Feature flags successfully documented.
The references to label_printer and thermal_printer under “features” are clear, ensuring users can easily enable or disable them.
207-215: Comprehensive documentation for label printer configuration.
The step-by-step explanation of each parameter (webhook, run_server, additional params) helps users set up label printing easily. Good job!
|
Is there a specific reason why this isn't merged yet? |
|
@frenck Something missing for the merge? |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
grocy/DOCS.md(2 hunks)grocy/config.yaml(3 hunks)
🧰 Additional context used
🪛 LanguageTool
grocy/DOCS.md
[style] ~226-~226: Consider using “who” when you are referring to a person instead of an object.
Context: ...to '/dev/ttyS0' Make sure that the user that runs the webserver has permissions to w...
(THAT_WHO)
🔇 Additional comments (3)
grocy/config.yaml (1)
34-35: Configuration structure looks solid.The printer feature toggles and tweaks are properly defined with consistent naming, correct schema types, and valid ranges. The port validation
int(1,65535)?follows best practices.Also applies to: 63-64, 76-85
grocy/DOCS.md (2)
174-175: Feature list documentation is clean.The new printer features are properly added to the features list following existing format.
207-225: Printer documentation is clear and well-structured.Good use of markdown link for ESC/POS protocol reference, and the option descriptions are detailed and helpful.
There was a problem hiding this comment.
Pull request overview
This PR adds support for label printer and thermal printer configuration options that grocy already supports but were not previously exposed in the Home Assistant add-on.
Key changes:
- Added feature flags for label printer and thermal printer with configurable options
- Exposed printer-related environment variables to grocy via the PHP-FPM run script
- Added comprehensive documentation for both printer types
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run | Added environment variable exports for label printer and thermal printer configuration options |
| grocy/config.yaml | Added schema and default values for label printer and thermal printer features and their respective tweak options |
| grocy/DOCS.md | Added documentation sections describing label printer and thermal printer configuration parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (1)
9-12: Printer tweak env wiring looks good; verify webhook env var nameThe new declarations and exports for printer tweaks (
GROCY_LABEL_PRINTER_RUN_SERVER,GROCY_LABEL_PRINTER_PARAMS,GROCY_LABEL_PRINTER_HOOK_JSON,GROCY_TPRINTER_IP,GROCY_TPRINTER_PORT, quantity-name and notes flags) are consistent withconfig.yamland follow the existing pattern for other tweaks. That part looks solid.One thing to double-check is the name of the webhook variable:
label_printer_webhook=$(bashio::config 'tweaks.label_printer_webhook') export LABEL_PRINTER_WEBHOOK=${label_printer_webhook}Given the rest of the env naming (
GROCY_*), you may actually want:-if bashio::config.has_value 'tweaks.label_printer_webhook'; then - label_printer_webhook=$(bashio::config 'tweaks.label_printer_webhook') - export LABEL_PRINTER_WEBHOOK=${label_printer_webhook} -fi +if bashio::config.has_value 'tweaks.label_printer_webhook'; then + label_printer_webhook=$(bashio::config 'tweaks.label_printer_webhook') + export GROCY_LABEL_PRINTER_WEBHOOK=${label_printer_webhook} +fiassuming Grocy reads
GROCY_LABEL_PRINTER_WEBHOOKlike the other settings. Please confirm against the upstream Grocy config/env docs before changing.Which env var does Grocy use for the label printer webhook: `LABEL_PRINTER_WEBHOOK` or `GROCY_LABEL_PRINTER_WEBHOOK`?Also applies to: 102-136
🧹 Nitpick comments (2)
grocy/DOCS.md (1)
207-227: Optional doc polish for printer option bulletsThe content is accurate, but you could tighten wording and punctuation for consistency with the rest of the doc (colon after option names, clearer wording on server/client side). For example:
-### Option: `Label Printer` - -Allows posting to a webhook to print labels - -- `label_printer_webhook` The URI that Grocy will POST to when asked to print a label -- `label_printer_run_server` Whether the webhook will be called server- or client-side -- `label_printer_params` Additional parameters supplied to the webhook -- `label_printer_hook_json`: TRUE to use JSON or FALSE to use normal POST request variables +### Option: `Label Printer` + +Allows posting to a webhook to print labels. + +- `label_printer_webhook`: The URI that Grocy will POST to when asked to print a label. +- `label_printer_run_server`: Whether the webhook is called server-side (from the add-on container) or client-side (from the browser). +- `label_printer_params`: Additional parameters supplied to the webhook. +- `label_printer_hook_json`: Set to `true` to use JSON, or `false` to use form-encoded POST variables. @@ -- `tprinter_print_quantity_name` Set to false if you do not want to print the quantity names (related to the shopping list) -- `tprinter_print_notes` Set to false if you do not want to print notes (related to the shopping list) -- `tprinter_ip` IP of the network printer -- `tprinter_port` Port of the network printer +- `tprinter_print_quantity_name`: Set to `false` if you do not want to print the quantity names (related to the shopping list). +- `tprinter_print_notes`: Set to `false` if you do not want to print notes (related to the shopping list). +- `tprinter_ip`: IP address of the network printer. +- `tprinter_port`: TCP port of the network printer.Purely cosmetic; feel free to keep as-is if you prefer.
grocy/config.yaml (1)
76-83: Consider making printer tweak fields optional or providing defaultsThese printer-related tweak fields are required in the schema (
str/bool/int(...)without?) but have no defaults underoptions.tweaks. That can make configuration noisy for users who never enable printer features, and the runtime already handles “unset” reasonably viabashio::config.has_value/true/false.Consider either:
- Marking them optional, e.g.:
orlabel_printer_webhook: str? label_printer_run_server: bool? label_printer_params: str? label_printer_hook_json: bool? tprinter_print_quantity_name: bool? tprinter_print_notes: bool? tprinter_ip: str? tprinter_port: int(1,65535)?- Adding sane defaults to
options.tweaksso the required fields are always populated.This would better match the pattern used for other optional tweaks like
calendar_first_day_of_week.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
grocy/DOCS.md(2 hunks)grocy/config.yaml(3 hunks)grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run(3 hunks)
🔇 Additional comments (3)
grocy/DOCS.md (1)
174-175: Printer feature flags documentation aligns with config and runtimeThe added
label_printerandthermal_printerentries in the features list match the new options/schema fields and the startup script wiring. No issues here.grocy/config.yaml (1)
34-35: Printer feature toggles are wired consistently
options.features.label_printer/thermal_printerand the correspondingschema.featuresbooleans are consistent and match the new runtime checks in the PHP-FPM run script. Defaulting both tofalseis a safe choice.Also applies to: 63-64
grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (1)
51-58: Original review comment is incorrect—the code is already using Grocy's correct naming convention.The correct environment variable name is
FEATURE_FLAG_THERMAL_PRINTER(without GROCY_ prefix), as confirmed by Grocy's configuration documentation and changelog. The PR code already uses the correct name. Changing it toGROCY_FEATURE_FLAG_THERMAL_PRINTERwould break the feature.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| label_printer_webhook: str | ||
| label_printer_run_server: bool | ||
| label_printer_params: str | ||
| label_printer_hook_json: bool | ||
| tprinter_print_quantity_name: bool | ||
| tprinter_print_notes: bool | ||
| tprinter_ip: str |
There was a problem hiding this comment.
The schema fields label_printer_webhook, label_printer_params, tprinter_ip, label_printer_run_server, label_printer_hook_json, tprinter_print_quantity_name, and tprinter_print_notes are defined as required fields (no ? suffix) but have no default values in the options section. This will cause validation errors when users don't provide these optional printer settings. Consider marking these as optional with str? and bool? or providing default values in the options section.
| label_printer_webhook: str | |
| label_printer_run_server: bool | |
| label_printer_params: str | |
| label_printer_hook_json: bool | |
| tprinter_print_quantity_name: bool | |
| tprinter_print_notes: bool | |
| tprinter_ip: str | |
| label_printer_webhook: str? | |
| label_printer_run_server: bool? | |
| label_printer_params: str? | |
| label_printer_hook_json: bool? | |
| tprinter_print_quantity_name: bool? | |
| tprinter_print_notes: bool? | |
| tprinter_ip: str? |
| - `label_printer_webhook` The URI that Grocy will POST to when asked to print a label | ||
| - `label_printer_run_server` Whether the webhook will be called server- or client-side | ||
| - `label_printer_params` Additional parameters supplied to the webhook | ||
| - `label_printer_hook_json`: TRUE to use JSON or FALSE to use normal POST request variables |
There was a problem hiding this comment.
[nitpick] The formatting is inconsistent with other list items in the same section. Lines 211-213 use a plain description format without a colon, but this line includes a colon separator. For consistency, either remove the colon and reformat to match lines 211-213, or add colons to the other items.
| - `label_printer_hook_json`: TRUE to use JSON or FALSE to use normal POST request variables | |
| - `label_printer_hook_json` TRUE to use JSON or FALSE to use normal POST request variables |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: workflows / Build aarch64
- GitHub Check: workflows / Build amd64
🔇 Additional comments (6)
grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run (6)
9-12: LGTM!Variable declarations for printer configuration are properly declared and follow the existing pattern in the script.
102-105: LGTM!Label printer webhook configuration is correctly retrieved and exported.
107-114: LGTM!Label printer run server flag and params configuration are correctly implemented.
116-118: LGTM!Label printer hook JSON configuration is correctly implemented.
120-136: LGTM!All thermal printer configuration exports are correctly implemented and follow the established patterns.
51-53: Code pattern is correct—label printer feature defaults to disabled.The default for GROCY_FEATURE_FLAG_LABEL_PRINTER is disabled (false), which aligns with the opt-in pattern used here. The code correctly enables the feature only when explicitly configured as
true.
| if bashio::config.true 'features.thermal_printer'; then | ||
| export FEATURE_FLAG_THERMAL_PRINTER=1 | ||
| export GROCY_TPRINTER_IS_NETWORK_PRINTER=1 | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
Fix the feature flag naming convention.
Line 56 exports FEATURE_FLAG_THERMAL_PRINTER without the GROCY_ prefix, which is inconsistent with all other feature flags in the file (e.g., GROCY_FEATURE_FLAG_BATTERIES, GROCY_FEATURE_FLAG_CALENDAR, GROCY_FEATURE_FLAG_LABEL_PRINTER).
Apply this diff to fix the naming convention:
if bashio::config.true 'features.thermal_printer'; then
- export FEATURE_FLAG_THERMAL_PRINTER=1
+ export GROCY_FEATURE_FLAG_THERMAL_PRINTER=1
export GROCY_TPRINTER_IS_NETWORK_PRINTER=1
fiPlease also verify that Grocy expects GROCY_FEATURE_FLAG_THERMAL_PRINTER and that the opt-in pattern (setting to 1 when enabled) matches Grocy's default behavior.
🌐 Web query:
What is the correct environment variable name for thermal printer feature flag in Grocy?
💡 Result:
The feature flag is FEATURE_FLAG_THERMAL_PRINTER — exposed as an environment variable with the GROCY_ prefix, i.e. GROCY_FEATURE_FLAG_THERMAL_PRINTER. [1][2][3]
Sources:
- Grocy changelog (feature flag name). [1]
- grocy config (env var prefix GROCY_). [2]
- config-dist.php (TPRINTER / feature-flag settings). [3]
Fix the feature flag naming convention.
Line 56 exports FEATURE_FLAG_THERMAL_PRINTER without the GROCY_ prefix, which is incorrect. The correct environment variable name is GROCY_FEATURE_FLAG_THERMAL_PRINTER, consistent with all other feature flags in the file.
Apply this diff:
if bashio::config.true 'features.thermal_printer'; then
- export FEATURE_FLAG_THERMAL_PRINTER=1
+ export GROCY_FEATURE_FLAG_THERMAL_PRINTER=1
export GROCY_TPRINTER_IS_NETWORK_PRINTER=1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if bashio::config.true 'features.thermal_printer'; then | |
| export FEATURE_FLAG_THERMAL_PRINTER=1 | |
| export GROCY_TPRINTER_IS_NETWORK_PRINTER=1 | |
| fi | |
| if bashio::config.true 'features.thermal_printer'; then | |
| export GROCY_FEATURE_FLAG_THERMAL_PRINTER=1 | |
| export GROCY_TPRINTER_IS_NETWORK_PRINTER=1 | |
| fi |
🤖 Prompt for AI Agents
In grocy/rootfs/etc/s6-overlay/s6-rc.d/php-fpm/run around lines 55 to 58, the
feature flag is exported as FEATURE_FLAG_THERMAL_PRINTER but should follow the
GROCY_ prefix convention; change the exported environment variable name to
GROCY_FEATURE_FLAG_THERMAL_PRINTER (and keep export
GROCY_TPRINTER_IS_NETWORK_PRINTER=1) so it matches the other feature flags in
the file.
There was a problem hiding this comment.
Opps missed one, I agree with this suggestion
There was a problem hiding this comment.
@valadas, thank you for confirming! Looking forward to seeing this fixed in the next update. 👍
5 participants
Proposed Changes
Adds support for printer configuration that grocy already supports but are not exposed in the add-on.
Related Issues
There was no issue for that, happy to create one if needed.
Summary by CodeRabbit
New Features
Configuration
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.