Closed vulpes2 closed 3 months ago
Hi there @vulpes2. As just laid out in https://github.com/matmair/brother_ql-inventree/pull/51#issuecomment-2246295956 I really appreciate the effort here. This change will be gladly welcomed into the fork.
I would ask to only format new code with tools like ruff/black to keep backports of the PR as simple as possible.
When working in similar situations in the past I found the (VS Code) settings python.linting.ignorePatterns
or ignore
(see https://code.visualstudio.com/docs/python/settings-reference#_python-language-server-settings) helpful. From my experience, most IDEs and linters provide similar tools to improve the signal-to-noise ratio.
I am sorry that my intention regarding back portability was not stated explicitly, it only covers existing code. New additions can follow modern Python behaviour and styling habits.
Would it be fine if I rebased the feature changes without the formatting changes?
Yes! Please (at) me once this is ready for review.
@matmair All done, I've only reformatted models.py
since it's just a list of hardware info, everything else was left as-is.
I will review/test this with a few printers and report back in the next few days!
I finally got time to test this with a few printers and labels; it looks great. So far I have only tested with usb-based printers. Am I understanding this right that it will only work with USB printers?
This might also be possible on network printers, but I do not own such a device to test it, nor is brother_ql
designed for two-way communication with network printers. In the send function you can see it doesn't even try to get a response if the network backend is selected, so some additional refactoring would be required before we can even attempt to implement it.
Note: PT-P700 seems to use slightly different command parameters, so the cutter isn't being activated right now. Will fix this when I have more time.
Just for information, maybe it helps: I am currently figuring out the same thing. It seems to me PT-P700 and PT-P710BT use same command parameters for cutting as QL-800 because the cutter works for PT-P700 and PT-P710BT when running the command as model QL-800 instead of PT-P750W.
@mkildemaa I've pushed a fix for the PT-P700, it seems to work for me. Please test on other PT printers and report back if you have access to them. I don't mind addressing other bits in the same PR if I'm able to.
There are a few issues I've fixed:
conversion.py
is calling add_print()
with last_page
set to true for all pages. If a block of raster data is not the final page, the print command should be 0x0C
instead of 0x1A
. The correct sequence for a multi-page printing job is [raster data] 0x0C [raster data] 0x0C [raster data (final page)] 0x1A
. The incorrect implementation works fine on QL printers, but will waste a huge amount of tape on PT printers, because PT printers will feed a blank piece of tape at the beginning of every print job. A print job is considered as terminated once 0x0A
has been received.
The auto-cut bit is at the same offset for QL and PT printers, so it was never correctly set in the first place for PT printers. The documentation is misleading because they used different starting offsets.
Chain printing (exclusive to PT printers) is now disabled by default. When chain printing is enabled, the printer does not feed and cut the last printed page. This is a useful behavior for printing multiple labels as a long continuous strip (label can optionally be half-cut on supported printers), but should not be the default. P-Touch disables it by default, and there is no code to support it in brother_ql
, so it should remain disabled until support is implemented.
Uncompressed data was still not printing properly on my PT-P700, I was getting garbled graphics or blank labels. Turns out the compression mode must be set with every single print job on a supported printer, otherwise the compression mode is unknown to a printer that supports both. There is no default mode specified in any of the raster command reference manuals, so this should be specified for both QL and PT printers for better reliability.
Ran this today and get this error:
./cli.py -b linux_kernel discover
File "/app/brother_ql-inventree/brother_ql/./cli.py", line 61
logger.info(f"Probing device at {device["identifier"]}")
^^^^^^^^^^
SyntaxError: f-string: unmatched '['
Output from my test runs:
INFO:brother_ql.backends.helpers:Media Type: Continuous length tape
INFO:brother_ql.backends.helpers:Media Size: 12 x 0 mm
INFO:brother_ql:Found a label printer at: usb://0x04f9:0x209b/000L8Z690042 (model: QL-800)
...
...
INFO:brother_ql.backends.helpers:Media Type: Continuous length tape
INFO:brother_ql.backends.helpers:Media Size: 62 x 0 mm
INFO:brother_ql:Found a label printer at: usb://0x04f9:0x209b/000L8Z690042 (model: QL-800)
...
...
INFO:brother_ql.backends.helpers:Media Type: Die-cut labels
INFO:brother_ql.backends.helpers:Media Size: 62 x 29 mm
INFO:brother_ql:Found a label printer at: usb://0x04f9:0x209b/000L8Z690042 (model: QL-800)
...
...
INFO:brother_ql.backends.helpers:Media Type: Continuous length tape
INFO:brother_ql.backends.helpers:Media Size: 102 x 0 mm
INFO:brother_ql:Found a label printer at: usb://0x04f9:0x20a8/000F1Z265904 (model: QL-1110NWB)
I really think you should make this code a little bit more specific to the task at hand "detect printer and media type". Then create another PR on top of this one which has the PT cutter included, if you would like.
Cutting, auto-cutting, etc definitely seems out of scope?
I am also able to use this code within the python scripts instead of the CLI. It only worked for pyusb backend in my testing.
from brother_ql.backends.helpers import status
>>> status("usb://04f9:209b")
{'series_code': 52, 'model_code': 56, 'status_type': 'Reply to status request', 'phase_type': 'Waiting to receive', 'media_type': 'Die-cut labels', 'media_width': 62, 'media_length': 29, 'errors': []}
>>> status("/dev/usb/lp1")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/app/brother_ql-inventree/brother_ql/backends/helpers.py", line 139, in status
result = interpret_response(data)
^^^^^^^^^^^^^^^^^^^^^^^^
File "/app/brother_ql-inventree/brother_ql/reader.py", line 165, in interpret_response
raise NameError('Insufficient amount of data received', hex_format(data))
NameError: ('Insufficient amount of data received', '')
It looks like python 3.12 has extended the f-string syntax so it was valid syntax only in python 3.12. Tested the fix with python 3.11 and everything seems to work. https://docs.python.org/3/whatsnew/3.12.html
About mixing the cutter related changes, I have very limited time to work on this and this PR is moving very slowly because none of us are doing this for a living, so I'd rather put it here instead of constantly rebasing a second PR just for those changes.
Unfortunately I also can't reproduce the other error you have been seeing:
>>> status("usb://04f9:2042")
{'series_code': 52, 'model_code': 53, 'status_type': 'Reply to status request', 'phase_type': 'Waiting to receive', 'media_type': 'Continuous length tape', 'media_width': 62, 'media_length': 0, 'errors': []}
>>> status("/dev/usb/lp0")
{'series_code': 52, 'model_code': 53, 'status_type': 'Reply to status request', 'phase_type': 'Waiting to receive', 'media_type': 'Continuous length tape', 'media_width': 62, 'media_length': 0, 'errors': []}
@matmair I believe all the concerns have been addressed, can you review this again? Would be great if we can get this merged soon.
@vulpes2 I am very happy with this, thank you for your contribution. Do you want to tackle #55 before I cut a new release?
@mkildemaa I've pushed a fix for the PT-P700, it seems to work for me. Please test on other PT printers and report back if you have access to them. I don't mind addressing other bits in the same PR if I'm able to.
There are a few issues I've fixed:
conversion.py
is callingadd_print()
withlast_page
set to true for all pages. If a block of raster data is not the final page, the print command should be0x0C
instead of0x1A
. The correct sequence for a multi-page printing job is[raster data] 0x0C [raster data] 0x0C [raster data (final page)] 0x1A
. The incorrect implementation works fine on QL printers, but will waste a huge amount of tape on PT printers, because PT printers will feed a blank piece of tape at the beginning of every print job. A print job is considered as terminated once0x0A
has been received.- The auto-cut bit is at the same offset for QL and PT printers, so it was never correctly set in the first place for PT printers. The documentation is misleading because they used different starting offsets.
- Chain printing (exclusive to PT printers) is now disabled by default. When chain printing is enabled, the printer does not feed and cut the last printed page. This is a useful behavior for printing multiple labels as a long continuous strip (label can optionally be half-cut on supported printers), but should not be the default. P-Touch disables it by default, and there is no code to support it in
brother_ql
, so it should remain disabled until support is implemented.Auto-cut definitions from the command reference
Sorry for the delay in answering. Much appreciated! All my printers are in the office in which the next time I will be there is tomorrow. I see the PR is now merged so I will report back only if there are any issues and if there is silence from my part then everything worked.
Do you want to tackle #55 before I cut a new release?
That would be ideal, thanks.
I've implemented some rudimentary printer model and media detection stuff. This should be a good base for adding support for media verification and printer model detection before printing later on. Tested working on Linux with a QL-700 and PT-P700.
Changes made:
Fixes pklaus#123 A good staring point for addressing #39 Fixes #54