fix(qrcode): adjust pixel width handling for QR code generation #102
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Ensure the generated QR code width does not exceed the requested width - Update the pixel width assignment logic to use the generated width when necessary - Add node_modules to .gitignore to prevent unnecessary files from being tracked Signed-off-by: Adrián Constante <41898282+github-actions[bot]@users.noreply.github.com>
- Introduce new constants for paper widths (58mm and 80mm) - Change default error correction level from Q to M - Update QR options struct and related methods to reflect new naming conventions - Ensure compatibility with existing QR printing functionality Signed-off-by: Adrián Constante <41898282+github-actions[bot]@users.noreply.github.com>
Contributor
|
👋 Thanks for opening this PR, @adcondev! Here's what will happen next:
Please make sure:
|
github-actions
bot
added
github
graphics
qr-image
executor
codeql
constants
size/M
labels
Contributor
⚡ Benchmark Results📈 Performance Comparison📊 Click to expand detailed resultsCurrent Branch ResultsBase Branch Results💡 Note: Use 🎯 Summary
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors QR code configuration and printing to use a new QrOptions struct, makes QR sizing depend on printer profiles, and adjusts defaults (notably error correction) for more predictable QR output.
Changes:
- Renames
QROptionstoQrOptionsand threads the new type through the service layer, executor, and graphics subsystem. - Makes QR maximum pixel width configurable per printer profile and improves pixel width clamping and image fallback behavior.
- Updates QR defaults (error correction level and pixel width handling) via new constants and handler logic, and fixes minor tooling/config issues.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/service/printer_service.go |
Updates PrintQR / printQRNative / printQRAsImage to use graphics.QrOptions, set a profile-based MaxPixelWidth, and adjust bitmap rendering based on generated image width. |
pkg/service/printer_mock.go |
Aligns the mock printer’s PrintQR signature with the new graphics.QrOptions type. |
pkg/service/printer_interface.go |
Updates the PrinterActions interface to use *graphics.QrOptions for PrintQR. |
pkg/graphics/qr.go |
Renames QROptions → QrOptions, introduces MaxPixelWidth, refines pixel-width validation and module-size calculations, and sets new defaults (error correction M, max width from paper pixel width). |
pkg/document/executor/qr_handler.go |
Uses graphics.DefaultQROptions() with new QR defaults and constants, and remaps string correction levels through constants.ErrorCorrection values. |
pkg/constants/defaults.go |
Changes DefaultQrErrorCorrection from Q to M and keeps QR-related defaults centralized. |
pkg/constants/constants.go |
Adds millimeter-based paper-width constants (Paper58mm, Paper80mm) alongside existing pixel-width constants. |
.gitignore |
Ignores node_modules to avoid committing Node-based tooling artifacts. |
.github/codeql/codeql-config.yml |
Fixes a glob pattern typo to correctly ignore *.exe build artifacts in CodeQL analysis. |
Comments suppressed due to low confidence (1)
pkg/document/executor/qr_handler.go:73
handleQRgained additional behavior around default error-correction and pixel width (viaconstants.DefaultQrErrorCorrectionandconstants.DefaultQrPixelWidth), but the existing tests inqr_handler_test.goonly cover JSON parsing and struct defaults, not the handler logic itself. Given this file already has tests, consider adding handler-level tests that exercise the default correction level, validation of invalidcorrectionvalues, and the new default pixel width so future changes to these constants can be caught by the test suite.
// Mapear corrección de errores (default: Q según schema)
correction := constants.DefaultQrErrorCorrection.String()
if cmd.Correction != "" {
correction = strings.ToUpper(cmd.Correction)
}
switch correction {
case constants.L.String():
opts.ErrorCorrection = posqr.LevelL
case constants.M.String():
opts.ErrorCorrection = posqr.LevelM
case constants.Q.String():
opts.ErrorCorrection = posqr.LevelQ
case constants.H.String():
opts.ErrorCorrection = posqr.LevelH
default:
return fmt.Errorf("invalid QR correction level: %s (valid: L, M, Q, H)", cmd.Correction)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
54
to
72
| } | ||
|
|
||
| // Mapear corrección de errores (default: Q según schema) | ||
| correction := "Q" | ||
| correction := constants.DefaultQrErrorCorrection.String() | ||
| if cmd.Correction != "" { | ||
| correction = strings.ToUpper(cmd.Correction) | ||
| } | ||
|
|
||
| switch correction { | ||
| case "L": | ||
| case constants.L.String(): | ||
| opts.ErrorCorrection = posqr.LevelL | ||
| case "M": | ||
| case constants.M.String(): | ||
| opts.ErrorCorrection = posqr.LevelM | ||
| case "Q": | ||
| case constants.Q.String(): | ||
| opts.ErrorCorrection = posqr.LevelQ | ||
| case "H": | ||
| case constants.H.String(): | ||
| opts.ErrorCorrection = posqr.LevelH | ||
| default: | ||
| return fmt.Errorf("invalid QR correction level: %s (valid: L, M, Q, H)", cmd.Correction) |
There was a problem hiding this comment.
The comment // Default: 128 no longer matches the behavior here: when pixel_width is omitted, the handler now sets opts.PixelWidth = constants.DefaultQrPixelWidth, which is currently 256, while the JSON schema (api/v1/document.schema.json, QRCommand.pixel_width.default) also documents 128 as the default. To avoid confusing API users, please either restore the 128px default or update both this comment and the schema default to match the actual value used by the handler (and keep them in sync going forward).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Description
This pull request introduces significant improvements and refactoring to QR code generation and printing logic, focusing on consistency, configurability, and code clarity. The main changes include standardizing the naming of QR code options, making error correction levels and maximum pixel widths configurable based on printer profiles, and improving the handling of QR code sizing and validation.
QR Code Options Refactoring and Consistency:
QROptionstoQrOptionsthroughout the codebase for consistency and clarity. Updated all relevant function signatures, struct names, and references. [1] [2] [3] [4] [5] [6] [7] [8]Configurable QR Code Settings:
Paper58mm,Paper80mm) and ensured that maximum QR code pixel width is set dynamically based on the printer's paper width profile. [1] [2]Improved QR Code Generation and Validation:
QrOptionsto clamp values within printer-specific limits and provide detailed logging if adjustments are made.Printer Service Enhancements:
QrOptions, dynamically set maximum pixel width, and provide more robust fallback between native and image QR code printing.Other Minor Improvements:
These changes improve the maintainability of the QR code subsystem, make it easier to add support for new printer types, and ensure more predictable and configurable QR code output.
🎯 Type of Change
🎯 Component(s) Affected
composer- ESC/POS protocol composerconnection- Printer connectionscommands- ESC/POS command implementationsbuilder- Document builderexecutor- Document processinggraphics- Image processingtables- Table formatting and layoutemulator- Visual emulatorprofile- Printer profilesservice- High-level printer servicegithub- GitHub related files and workflows🧪 How Has This Been Tested?
Test Configuration
🤖 Automated Checks
The following automated workflows will run on this PR:
✅ Checklist
📸 Screenshots/Examples
Click to expand
// Example code here💥 Breaking Changes
🔗 Related Issues
📝 Additional Notes
📚 Resources: