Another batch of changes

[tomers]

Each commit does something else, so please read each one separately

[maresb]

Thanks @tomers!

I'm not quite following what is VERTICAL_PREVIEW_MARGIN_PX. Could you explain it?

Is the idea that for the preview we add a fake margin for the nonprintable area so that the preview resembles the physical tape after printing?

[maresb]

That's really nice with the adjustable margin in the preview. But it's a bit weird that the margin code for the preview is different from the margin code for the printer.

Looking more closely I see that the printer code prints no margin at the beginning and a double-margin at the end. I think this probably has to do with the gap between the printer head and the cutter. I'm guessing that the gap is approximately DEFAULT_MARGIN_PX pixels long. Then when we print 2 * DEFAULT_MARGIN_PX, where the first DEFAULT_MARGIN_PX is the true right-margin of the current label, and the second DEFAULT_MARGIN_PX is the left-margin of the next label. Do you agree with this assessment?

It might be nice to start distinguishing some of these concepts in the code. Like have a constant for GAP_BETWEEN_PRINT_HEAD_AND_CUTTER_PX = 13, and have separate variables in the code, something like current_right_margin and future_left_margin (or whatever makes sense), just to make the code somewhat consistent with reality.

In principle we could enable a margin-free mode where you print the first GAP_BETWEEN_PRINT_HEAD_AND_CUTTER_PX pixels, tell the user to cut, and print the remaining pixels plus future margin, and tell the user to cut again.

If the user wants a label with margin greater than GAP_BETWEEN_PRINT_HEAD_AND_CUTTER_PX, then I think the correct way to do this is:

• Print a left margin of additional_left_margin = margin - GAP_BETWEEN_PRINT_HEAD_AND_CUTTER_PX, since we already have GAP_BETWEEN_PRINT_HEAD_AND_CUTTER_PX due to the gap from the previous label.
• Print the main payload
• Print a right margin of margin
• Continue printing a margin of future_left_margin = GAP_BETWEEN_PRINT_HEAD_AND_CUTTER_PX. (I think the current implementation is buggy since it'll produce too short of a left-margin and too long of a right-margin, but I'm not able to test at the moment.)

Am I making sense?

[tomers]

@marseb, thanks for the insights. I've contrinued to modify the code, and I am going to introduce some further changes soon. Thanks for the insight of explaining why we print the margins twice at the end. I was puzzled by that. I will add some of your text as comments in the code, and I also consider taking the variable and constant names you suggest. Let's keep this PR open for a while, until I'm done with that.

[maresb]

Awesome, thanks so much for looking into that!!!

I've converted the PR to draft for now. When you're finished just hit "Ready for review" and I'll give it another look and merge if I approve.

[tomers]

Exactly. I still need to work on that part, but the idea is that preview should show both horizontal and vertical margins.

On Sat, Feb 3, 2024, 12:38 Ben Mares @.***> wrote:

Thanks @tomers https://github.com/tomers!

I'm not quite following what is VERTICAL_PREVIEW_MARGIN_PX. Could you explain it?

Is the idea that for the preview we add a fake margin for the nonprintable area so that the preview resembles the physical tape after printing?

— Reply to this email directly, view it on GitHub https://github.com/computerlyrik/dymoprint/pull/113#issuecomment-1925271482, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUL46ACLSJ36PFVNAFMEDYRYHRDAVCNFSM6AAAAABCXDDOQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVGI3TCNBYGI . You are receiving this because you were mentioned.Message ID: @.***>

[maresb]

Okay, great!

Keep in mind that there are a few different levels of abstraction coming together:

• the bitmap itself that's to be printed
• the way the bitmap is printed onto a piece of tape

I think there are valid use cases to have preview functionality for both. Unfortunately I don't have any clever ideas for an implementation.

Note that this is starting to connect up with @tomek-szczesny's work on a lookup table for various printer and label width combinations in #91.

It's really great how things are coming together thanks to your work @tomers! Back when #91 was opened, I don't think the code was healthy enough to carry through that implementation. But now the code is becoming much easier to understand, opening up lots of exciting possibilities!

[tomek-szczesny]

Certainly I'm glad someone's cleaning up my amateur code. :) Just wanted to add, I regret that printer must be detected before rendering, in order to determine the actual printable area. I see no other way around it, barren the user-provided printer model as a command line parameter.

[maresb]

my amateur code

To be clear the obstacle wasn't your code, it was instead the underlying code which was very idiosyncratic with not-so-great separation of concerns and idiosyncratic variable names. Those sorts of things build up and create a tremendous amount of friction for making changes.

[tomers]

Thanks, @maresb and @tomek-szczesny! I'm working on discarding margin handling in the printing driver. This adds mental load on understanding the code, and the principle of having to end the print job with an offset for the next print job, and will be a burden in the future, when we'll have multiple drivers (which would need to duplicate this task), and also in the case where we may want to implement some more complicated scenarios, such as printing batch of labels, with zero margin (i.e. print brank without ending with offset, cut the label, then iteratively printing labels without margins, or at least with SW defined margins, and instructing user to cut).

@maresb, regarding your comment about idiosyncratic code, that's indeed true, and the was my initial effort, to clean these things off the table, before approaching any other new features.

[tomers]

Hey @maresb and @tomek-szczesny,

Can you please review this PR?

Thanks, Tomer

[maresb]

Oh, looks like you're actively working on this. I'll hold off on further review for the moment.

[maresb]

Are you okay with the conflicts there? Looks like maybe you accidentally rebased on a stale branch?

[tomers]

It is ready for review now. Sorry, but I use git rebase a lot, and I keep forgeting to rebase on origin/master.

[tomers]

Any news?

[maresb]

Sorry, I'm in a time crunch at the moment. It might have to wait another few days.

[tomers]

Hey there,

Due to irresponsivenes, I'm abandoning this project, and will fork it in a new organization, managed by me. Whoever wants to join me, is most welcome to contact me via LinkedIn.

[maresb]

Dang, that's pretty drastic. As you wish.

[tomers]

I hope I did not sound too drastic. That was not the intention. I mean that I will cease development on that repository in the future. I have no hard feelings at all! It is simply that I can't wait 3 weeks for a PR to be merged... It's hard to work like that. Note that my action is aligned to what we've discussed earlier this month on LinkedIn. We agreed that we should open a separate, new, Github organization for this purpose.

On Wed, Feb 28, 2024 at 10:04 PM Ben Mares @.***> wrote:

Dang, that's pretty drastic. As you wish.

— Reply to this email directly, view it on GitHub https://github.com/computerlyrik/dymoprint/pull/113#issuecomment-1969774315, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUL4Y2Q352EJWMSIT3ZI3YV6EUNAVCNFSM6AAAAABCXDDOQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRZG43TIMZRGU . You are receiving this because you were mentioned.Message ID: @.***>

[maresb]

Ya, understandable. I'm going through a very stressful period at the moment, and unfortunately I haven't had any time in the past few weeks to commit to open-source.

I would have expected such an announcement to be a bit more coordinated and communal, not so unilateral. But you seem to have a lot of energy, so I think it's good that you to take charge.

[tomek-szczesny]

This repository is already abandoned by its actual owner so it does make sense to fork it into a new entity that will be easier to pass control to. I remember that @maresb had problems with privileges here already.

I think a link to the new repo / organization should be added on top of README here, and this repository archived if possible.

[tomers]

I would have expected such an announcement to be a bit more coordinated and communal, not so unilateral I totally agree with that. I am sorry for being hasty with my response. Ben and Tomek, maybe we should find some time for a short discussion (this medium is bad for that) about how we're going to push this project forward. Alternatively, if you approve, I can open a new Github organization and fork this project (I will also update README accordingly).

On Thu, Feb 29, 2024 at 6:43 AM Tomek Szczęsny @.***> wrote:

This repository is already abandoned by its actual owner so it does make sense to fork it into a new entity that will be easier to pass control to. I remember that @maresb https://github.com/maresb had problems with privileges here already.

I think a link to the new repo / organization should be added on top of README here, and this repository archived if possible.

— Reply to this email directly, view it on GitHub https://github.com/computerlyrik/dymoprint/pull/113#issuecomment-1970385605, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUL47UKIZC3HZ7OP7FPELYV2YVZAVCNFSM6AAAAABCXDDOQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZQGM4DKNRQGU . You are receiving this because you were mentioned.Message ID: @.***>

[tomek-szczesny]

I don't feel there is anything left to discuss, a new organization and fork of this repository is reasonable and I think everyone has already expressed approval. Just let us know so we can join it. :)

[maresb]

I wanted to send an email to recent contributors, and also @computerlyrik, letting them know about the plan, and also to solicit a new name for the fork. There are also some logistical issues like how to communicate the deprecation of this project. I think a quick call may be useful to sort out these details. @tomek-szczesny, it'd be nice to have you there too, but @tomers and I could probably make do with just the two of us.

[tomers]

@tomek-szczesny, @maresb, let's continue this discussion in issue #114 .

[maresb]

Some preliminary review to fix some breakage that I'm seeing: https://github.com/tomers/dymoprint/pull/6

Here's the output of dymoprint --preview asdf on v2.4.0:

Here's the output on https://github.com/tomers/dymoprint/pull/6

I can't even take a screenshot of the full output. I understand that we're enlarging the preview to simulate the printable area, but I'm wondering what all this extra black space is about.

[tomers]

Hello @maresb , thank you for reviewing my code! :-) Apologies for the delay in addressing the issues raised. Your feedback guided me towards better solutions, and I appreciate your patience throughout the process.

[maresb]

More image mode headaches :dizzy_face:

$ dymoprint --browser asdf --verbose
[DEBUG] Read config file: /home/mares/.config/dymoprint.ini
[DEBUG] Demo mode: showing label..
[ERROR] not supported for this image mode
Traceback (most recent call last):
  File "/home/mares/repos/dymoprint/src/dymoprint/lib/utils.py", line 49, in system_run
    yield
  File "/home/mares/repos/dymoprint/src/dymoprint/cli/cli.py", line 343, in main
    run()
  File "/home/mares/repos/dymoprint/src/dymoprint/cli/cli.py", line 325, in run
    ImageOps.invert(bitmap).save(fp)
  File "/home/mares/micromamba/envs/dymoprint-dev/lib/python3.8/site-packages/PIL/ImageOps.py", line 563, in invert
    return image.point(lut) if image.mode == "1" else _lut(image, lut)
  File "/home/mares/micromamba/envs/dymoprint-dev/lib/python3.8/site-packages/PIL/ImageOps.py", line 60, in _lut
    raise OSError(msg)
OSError: not supported for this image mode

[maresb]

The issue with the huge preview margins described in https://github.com/computerlyrik/dymoprint/pull/113#issuecomment-1975153786 is resolved. :rocket:

[tomers]

Done with the fixes, @maresb

[maresb]

--browser now works again. There are still a few things unresolved from the last review.

It's really difficult to review such a large PR after a rebase, so could you please refrain from rebasing until after I've approved?

[tomers]

Thanks Ben,

The branch is ready to be merged from my side. I am not going to change it anymore. You can merge according to the plan that we discussed privately.

On Sun, Mar 24, 2024 at 8:13 PM Ben Mares @.***> wrote:

@.**** approved this pull request.

Whew, that was a whole lot of work, but I think we finally got it. Thanks @tomers https://github.com/tomers for persevering. These are some really huge improvements.

Go ahead and rebase if you'd like, and then let's get this merged.

— Reply to this email directly, view it on GitHub https://github.com/computerlyrik/dymoprint/pull/113#pullrequestreview-1956616353, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAUL43WPD4PD2HOEJXUNDLYZ4JVBAVCNFSM6AAAAABCXDDOQ6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSNJWGYYTMMZVGM . You are receiving this because you were mentioned.Message ID: @.***>

[maresb]

Thanks @tomers!

For anyone reading, to avoid giving the impression of being secretive, the plan is to not release this as DymoPrint, but instead release it as Labelle v1.1.0 (see #114). And Labelle v1.0.0 will be a rebranded DymoPrint v2.4.0.