Device dependent margins/config

[FaBjE]

Closes #51

I have added a DeviceConfig object that contains a configuration for each type of printer/labeler. I started out with the lookup table, but puzzeled by the "random" margins I figured it out how the principal works.

For every printer we now need a "print head size in pixels" and "print head active size in mm" the rest is calculated. I think there might only be two versions (128px/18mm and 64px/9mm).
Based on my scans we determined a label-position inaccuracy of 0.7mm. I rounded it up to 1mm to use as safety margin for now.

I´d like to hear your thoughts. Bear in mind that this is my first python code. So you may find a lot of weird things... This is not done yet, so only a draft PR.

I've tested it on my LabelManager PC II and a LabelManager PnP both can print valid labels 12mm (and 24mm on the PCII)

I have identified a couple of issues so far:

• Supported tape sizes dropdown not cleared in GUI, making a mess. With the config object it should now show only the options supported by the printer
• Margins preview in GUI are not correct
• Boxed text, left border 1px missing
• mm_to_px functions are maybe not accurate. Look into/refactor those?

[maresb]

Thanks @FaBjE for the awesome contribution and @tomers for such a detailed review! I'm pretty excited about finally getting this feature settled.

I agree that we'll want to do another round of review after addressing these structural and stylistic issues.

If possible, try to rebase fixes onto existing commits, to me review easier on us.

I'm wondering what is exactly your preference here @tomers. Most reviewers prefer to avoid rebasing until the end of the review process so that there is a clear history of the changes. However, GitHub introduced a feature that makes it easy to diff before and after a rebase, so this is no longer such a big deal. I don't have such a strong preference, but I'd like to understand your rationale. (For tiny PRs I slightly prefer rebasing, but with larger PRs like this one I like new commits.)

[tomek-szczesny]

(Comment copied from the issue)

Hm. The point of LUT that I developed was to be prepared for any future Dymo printer. That is why I proposed to measure print area in dots, as well as a single necessary offset, and then only name this settings in mm for user's convenience. No millimeters handled in the code because frankly we don't need them there at all.

I'm afraid that although your solution is convenient for supporting a subset of Dymo products known to us at the moment, I'm not sure how well this will scale with printers of other brands or series.

[tomers]

I'm wondering what is exactly your preference here @tomers. Most reviewers prefer to avoid rebasing until the end of the review process so that there is a clear history of the changes. However, GitHub introduced a feature that makes it easy to diff before and after a rebase, so this is no longer such a big deal. I don't have such a strong preference, but I'd like to understand your rationale. (For tiny PRs I slightly prefer rebasing, but with larger PRs like this one I like new commits.)

I am not orthodox on this, so I guess I can live with whatever approach used. I guess it is all about the convenience of the author and reviewers. If the author doesn't feel comfortable with rebasing, let him do whatever he wants, as to not put a burden on him. I am not familiar with Github tooling that much. I personally don't like the review system, as it is not straight forward to my opinion, and makes it hard to track progress. In my work we use https://rbcommons.com/ which is a good tool to monitor changes to a given commit.

As you saw in the past, I personally prefer to keep contained list of commits, and keep rebasing the same commits over and over, until the change is stable, and each commit containes a specific subset of the total work. In this approach you will not encounter TODO section, and then a removal of that TODO in a subsequent commit.

I agree that the approach depends on the size of the given review, and the number of iterations in its lifetime. Using the rebase approach indeed makes it harder for reviewers to track changes in the review.

So I'll leave it for @FaBjE to decide how he would like to work on this review. I want him to enjoy his work, and keep his motivation to contribute to the project :-)

[FaBjE]

@All Thank you for your feedback guys I completely understand a lot of style issues. I'm not happy with most of them. But it are either issues of me not knowing python syntax well enough or the linter/validator settings not allowing me to. let me address your points:

Hm. The point of LUT that I developed was to be prepared for any future Dymo printer. That is why I proposed to measure print area in dots, as well as a single necessary offset, and then only name this settings in mm for user's convenience. No millimeters handled in the code because frankly we don't need them there at all.

I'm afraid that although your solution is convenient for supporting a subset of Dymo products known to us at the moment, I'm not sure how well this will scale with printers of other brands or series.

I get your point and to emphasize it. I started out with exactly your solution. But I got stuck and it made me rethink it. I think I have settled in "middle ground" but let me explain: In the software there are two units mixed at the moment. Pixels (dots) and millimeters. There is a "pixels per millimeter" constant somewhere. But I doubt it's validity.

The thing is the tape sizes are only known in millimeters. You could make this in pixels. But than I wondered, how many pixels is a particular tape size. Well, that depends on the printer. (thinking about the future) it depends on the pixels-per-millimeter of the print head. So I need to know the amount of pixels in the print-head. Than my calculations didn´t workout. Why? Because the print-head pixels don´t match the largest tape-size it can print. The PC II printhead active area is only 18mm as tested with your test-pattern. (I feel slightly scammed with my 24mm labels now, as they will never be printed full-size 🤔 ) So I added that configuration parameter as well.

I was able to directly determine tape size (in pixels) now. and I started working on the offets LUT. But not having any 6mm tape for example, I had to calculate them. and than it occurred to me that I had all the input parameters of my calculation in software. So why not add the calculation there. It still uses the tuple group of pixels offsets there. I was thinking that if we encounter non-centered printers in the future we can add a (subset) of the LUT to the device configuration object. and make a "if not in LUT use calculation, otherwise use set value" kind of structure. Thinking we have the best of both worlds.

For my two test printers the calculation worked fine. So I left off at that point.

My expectation is that 90% of the printers will be valid, but there is always a exception. An other "augment" is that I want to make adding printers as easy as possible. The LUT is a bit harder to generate for an inexperienced programmer than just measuring the size of your test-pattern print.

I hope you can agree with me, but I'm open to your thoughts.

So I'll leave it for @FaBjE to decide how he would like to work on this review. I want him to enjoy his work, and keep his motivation to contribute to the project :-)

I'm not really sure what you are on to about rebasing. I'm used to just commiting all the changes (of review comments). At the end I should do some squashing as I made a pretty mess by now. but I leave that till later. In case other features are merged to master I rebase this featurebranch on master to be up to date again. Does that work for you?

[maresb]

I'm not happy with most of them. But it are either issues of me not knowing python syntax well enough or the linter/validator settings not allowing me to.

No worries! Please just do as well as you practically can, and we'll help out with the rest.

I agree that tape sizes are listed in mm, so for the UI it definitely makes a lot more sense than px. But personally I think of it more as a label. Like in my mental model, "12 mm" corresponds to the string "12 mm" rather than a numerical value 12.0 that can be computed with. As a result, doing a sanity check with mm to ensure the pixel counts are roughly correct is one thing, but it seems like a headache to get conversion and rounding to work out in a simultaneously simple and correct way.

The LUT is a bit harder to generate for an inexperienced programmer than just measuring the size of your test-pattern print.

Ya, I think the process could be made simpler, like enhancing the test pattern by stuff like adding numbers to the larger blocks, and to print some additional lines and checkerboard patterns. But we're not generating these LUTs at scale, so I'd rather personally help the inexperienced users to get things dialed in precisely with units of pixels. Does that make sense?

Please don't worry about the rebasing, just do whatever's easiest for you.

Thanks a lot for taking such an ambitious approach here! If this gets to be too much, please let us know and we can think about how to break it apart into smaller features.

[maresb]

I was thinking that if we encounter non-centered printers in the future

I thought we determined in #11 that your printer with the 9mm tape is non-centered with a top margin of 37 and a bottom margin of 27.

[FaBjE]

I agree that tape sizes are listed in mm, so for the UI it definitely makes a lot more sense than px. But personally I think of it more as a label. Like in my mental model, "12 mm" corresponds to the string "12 mm" rather than a numerical value 12.0 that can be computed with. As a result, doing a sanity check with mm to ensure the pixel counts are roughly correct is one thing, but it seems like a headache to get conversion and rounding to work out in a simultaneously simple and correct way.

True, but somewhere we must know the size of the label in pixels. I think we cannot assume that is a constant. So it should be calculated based on the printer properties, I'll keep it as is for now. Feel free to update it somewhere in the future.

The LUT is a bit harder to generate for an inexperienced programmer than just measuring the size of your test-pattern print.

Ya, I think the process could be made simpler, like enhancing the test pattern by stuff like adding numbers to the larger blocks, and to print some additional lines and checkerboard patterns. But we're not generating these LUTs at scale, so I'd rather personally help the inexperienced users to get things dialed in precisely with units of pixels. Does that make sense?

Yes and no, I think it is more a thing of preference. But I honestly think the calculation is not that hard. At the moment we have the same precision we would have with pixels.

Please don't worry about the rebasing, just do whatever's easiest for you.

Thanks a lot for taking such an ambitious approach here! If this gets to be too much, please let us know and we can think about how to break it apart into smaller features. It's OK for now. But I don´t have all the time in the world. So My take is to get the code to an acceptable level to get it merged. Than we can improve upon that "base"

I was thinking that if we encounter non-centered printers in the future

I thought we determined in #11 that your printer with the 9mm tape is non-centered with a top margin of 37 and a bottom margin of 27.

Yes I thought so too, but than I calculated the following: My printer is 128 pixels at 18mm. so 7.11p/mm 9 mm tape should be 63.99px, lets round to 64px. Leaving 64 pixels as margin, for a centered tape this should be 32 at both sides. 37-32= 5, and 27-32=-5, so the tape has deviated 5 pixels from the center. 5 / 7.11 = 0.70mm. So the tape is 0.7mm off-center. Playing with the tape cartridge a bit that would make sense as there is always a bit of play in the tape guides and position of the tape in the printer.

So in my opinion the 0.7mm (I rounded it to 1mm) is always the manufacturing tolerance we have to deal with. You can calibrate it pixel-perfect on my printer. But there is no guarantee that somebody else with a (Same model) different printer and different tape is aligned the exact same way.

[tomers]

Regarding px vs mm discussion, please refer to Dymo's LW 550 Technical Reference Document, where they describe the properties of their print head (page 4):

About the LabelWriter 550 Series Printers The LabelWriter 550 series printers (LabelWriter 550, 550 Turbo, 5XL) are high-performance, low-cost printers used for printing mailing labels, postage, file folder labels, barcode labels, and more. The LabelWriter 550 and LabelWriter 550 Turbo printers have a 57 mm wide print head. The LabelWriter 5XL has a 101 mm wide print head. All printers have 300-dpi print head. The 57 mm wide thermal print head uses 672 individually addressable dots to form individual raster lines of data at 300 dots per inch across the print head—the 101 mm wide print head uses 1248 dots. Both use 300 dots per inch in the travel direction as directed by the print control data.

Note how they describe both the printer head width, the density (in dpi), and the number of pixels. I think our inner configuration and logic should use pixels. User facing parts should be in millimeters.

[tomek-szczesny]

It's not a veto but I don't agree that LUTs with two numerical parameters are more complicated than anything else. Because that's what we ultimately need, a printout width in dots, and offset (bottom margin) in dots, for tapes smaller than a print head. Top margin isn't required. All the rest about my LUTs may have been suboptimal because I suck at Python, but certainly I wouldn't bother amassing any more information than two dot values of our concern.

The formula that you brought up could be used by us internally to get estimated LUT values in case none of us has a printer or tape of interest.

Disclaimer: I am heavily biased towards simplicity and minimal logic, because most of my coding is for FPGAs. ;)

The test pattern was designed so it gives all the necessary information in one go. The idea was that an owner of an unsupported printer would share a test pattern photo, and we would update the LUT based on that. Ben said he could do that and certainly I could too. I also promised to write a manual before we all inevitably step down from our posts here.

Millimeters were a nightmare of the legacy code and I made it a point to remove them from everywhere except UI strings. Yes, that conversion constant wasn't precise anyway, and will not be even remotely true once we turn our attention to other thermal printers. At the very least, if we decide to stick to millimeters in cases like print margins settings and so on, these should be defined separately for each printer. Printers with different resolutions across x and y axes are so rare I wouldn't worry about that.

[maresb]

Thanks for the changes @FaBjE! This is shaping up very nicely.

So the tape is 0.7mm off-center. Playing with the tape cartridge a bit that would make sense as there is always a bit of play in the tape guides and position of the tape in the printer.

So in my opinion the 0.7mm (I rounded it to 1mm) is always the manufacturing tolerance we have to deal with. You can calibrate it pixel-perfect on my printer. But there is no guarantee that somebody else with a (Same model) different printer and different tape is aligned the exact same way.

It makes a lot more sense to me now, thanks for the context!

I'm disturbed by a discrepancy in the technical reference where they claim 300 dpi = 11.811 px/mm. But @FaBjE computes and verifies 7.11 px/mm. Any ideas what's going on here?

I will try to give the changes a more thorough inspection/test later today.

[tomek-szczesny]

I measured my old test pattern from PnP, there are 48 dots across about 6.85mm -> 7.01/mm, or 178 dpi.

The most common thermal printer resolutions are 203dpi (8/mm), 300dpi and 600 dpi. I've never heard of thermal print heads below 200dpi.

My best guess is that they use >300dpi print head and do pixel doubling in firmware due to some problems found at the development stage.

[FaBjE]

Thanks for the changes @FaBjE! This is shaping up very nicely.

Not done yet 😉 But I won´t be able to finish everything today.

So the tape is 0.7mm off-center. Playing with the tape cartridge a bit that would make sense as there is always a bit of play in the tape guides and position of the tape in the printer. So in my opinion the 0.7mm (I rounded it to 1mm) is always the manufacturing tolerance we have to deal with. You can calibrate it pixel-perfect on my printer. But there is no guarantee that somebody else with a (Same model) different printer and different tape is aligned the exact same way.

It makes a lot more sense to me now, thanks for the context!

No problem.

I'm disturbed by a discrepancy in the technical reference where they claim 300 dpi = 11.811 px/mm. But @FaBjE computes and verifies 7.11 px/mm. Any ideas what's going on here?

The example with 300dpi is about the newer model printers. We have different older style printers. with about 180 dpi (180.6222)

I will try to give the changes a more thorough inspection/test later today.

About that, do I click the "Resolve conversation" button, or do you do that when my changes are verified?

[maresb]

About that, do I click the "Resolve conversation" button, or do you do that when my changes are verified?

Ideally the person making the request (asking a question, requesting a change) resolves. If it's clear that you've decisively addressed the request and no followup is needed, then you can close yourself. It's also fine to leave them open. There are no rules, only common sense and etiquette.

[FaBjE]

About that, do I click the "Resolve conversation" button, or do you do that when my changes are verified?

Ideally the person making the request (asking a question, requesting a change) resolves. If it's clear that you've decisively addressed the request and no followup is needed, then you can close yourself. It's also fine to leave them open. There are no rules, only common sense and etiquette.

Good to know. Makes sense to me, I've left most open as I think my fixed should be check too if they are the desired way. I think I addressed all comments now.

[FaBjE]

I did a big change for the DPI values. https://github.com/labelle-org/labelle/pull/54/commits/7246473e2914ffa7574c58155b7ca9147eba96a3 I'm not sure if I should make a separate PR for that, or include it here.

[FaBjE]

Thanks so much @FaBjE for your work here.

Ok, I'm going to be brutally honest here. Not that I want to, but I think it needs to happen.Up until this point I liked to contribute something to labelle and getting my label printer to work under linux. But this is starting to change.

The current codebase is/was not made for printers with different properties. (Supported tape sizes, dpi, etc) I tried to integrate this, but I needed to touch a lot of stuff.

Trying to to change as little as possible (within reason) I did this. But there is a lot of stuff I think that can be handled different/better with the things we know now. Some may be personal preference or coding style preference. In my opinion, either we accept it is not as pretty as we want it, but it works. Or we do a big overhaul of the code and make everything pretty.

But I think we are stuck in a no-mans-land between the two at this point.

As I said in the opening post of this PR this code is not perfect. There are issues with it, mostly caused by my changes "conflicting" the current setup of the codebase.

Let me address the points:

When I run labelle-gui with my LabelManager PnP, I get the following stack trace:

qt.gui.icc: fromIccProfile: failed size sanity 2
Traceback (most recent call last):
  File "/home/mares/repos/labelle/src/labelle/gui/gui.py", line 108, in _on_settings_changed
    height_px=self._dymo_labeler.label_height_px,
  File "/home/mares/repos/labelle/src/labelle/lib/devices/dymo_labeler.py", line 277, in label_height_px
    return self.tape_print_properties.usable_tape_height_px
  File "/home/mares/repos/labelle/src/labelle/lib/devices/dymo_labeler.py", line 305, in tape_print_properties
    raise ValueError(
ValueError: Unsupported tape size 24mm. Supported sizes: [6, 9, 12]mm
Aborted (core dumped)

I believe this is indicative of a structural problem that we are introducing here. (Although the rest of the codebase is not innocent.) I strongly disapprove of the use of properties here. Accessing class attributes should be a completely safe operation. (The type checker should guarantee that they're defined.) In the stack trace we see two nested attribute accesses, which is an insane obfuscation. Unless there are exceptional circumstances, attributes should be static and functions should compute stuff. Note also how tape_print_properties is a huge block of (thankfully well-documented) code.

Yes, this is caused by a structural problem the GUI has. For starters: I didn´t notice this as my printer supports all tapes. But the GUI can´t (properly) handle changing printers (properties).

More low level: when the printer properties are loaded (like supported tape sizes) the change-event of the control fires every time a item like a tape size is added/removed.

It starts with the default (simulator) config. (which has all supported tape sizes) Than the USB printer is initialized and "attached" to the GUI. Than the weird stuff/problems starts to happen. The "backend" already has the USB printer. But the GUI controls still have the old data. Change events fire, and the GUI tries to set properties (left over from the default) that the printer doesn´t support. So boom, exception.

IMHO: The change events should not fire when the printer properties are being loaded.1. Disable change events, 2. Clear GUI controls, 3. Load new printer properties to controls, 4. Enable change events.

Instead of having a property called label_height_px I'd like to instead see an ordinary function get_label_height_as_px. The benefits to this approach are that:

1. we define an honest interface where attributes are attributes and functions are functions

2. it makes us actually think about what should be an attribute vs a function instead of lazily reencoding everything imaginable as a property, creating needless layers of indirection

For starters, this label_height_px was always in the codebase. Originally it was named height_px as a property of the labeler object. Which is weird imho, because it is not the height of the labeler. (as I read it) but the tape height. So i did a rename.

Second, there already were a lot of properties that do either the same or something similar to eachother.

Third, I was specifically asked to make this a property. https://github.com/labelle-org/labelle/pull/54#pullrequestreview-2133618583 If you are 100% sure you want it a function I will convert it back to a function, but that will be the last time I'm changing it.

But then this also brings up the issue of whether or not we should even have mm in the first case.

A tape has:

* height in mm

Correct

* height in px

No. The PRINTER has a height in px for a tape. There are no pixels on a tape.

* dpi / pixels_per_mm

No. The PRINTER has a DPI.

  and any two determines the third. As far as I can tell, we don't need DPI for anything, and it doesn't seem like we even need height in mm if we treat "tape size" as a label. While we can do some clever stuff with dpi for DYMO printers, I wonder if it's actually worth all this code complexity.

You need DPI for the GUI (margins display) and CLI, calculations of tape size in pixels (for a particular printer) FYI: In my last commit the DPI from the CLI was set different than the DPI of the constants.

If I were to setup this code today, I would make the two of classes of 1: Printer Config object, containing static data of a printer 2: Printer runtime object, containing all functions of a printer and the "runtime" calculations. Of say for example the amount of pixels the printer can print on a (selected) tape size.

After that, pass the printer object to all classes that needs to to something. Like the renderers, GUI etc. They can retrieve the data they need from the printer object.

They may need some more input, like for example a text or barcode that needs to be rendered. But all printer/tape specific data can just be retrieved from the printer object instead of passing around lots of separate variables.

Well, these are just my two cents. I´d like you to define a strategy, prospect, view or design or whatever you want to call it how you want to continue. And I'll see if I want to be a part of that.

[tomers]

Third, I was specifically asked to make this a property. https://github.com/labelle-org/labelle/pull/54#pullrequestreview-2133618583 If you are 100% sure you want it a function I will convert it back to a function, but that will be the last time I'm changing it.

I suggested it, but I am not orthodox about it, and I do accept the reasoning raised earlier, as to why this was a bad suggestion. I am sorry for causing you to waste your eneregy, and lose some motivation to work on this codebase. I hope you'll continue your excellent work, and keep improving it. The architecture indeed needs some thoughts and design in order to make it better. I love the idea of separating contant printer data (print head specification, etc.) and runtime data (current tape loaded).

[maresb]

Hi @FaBjE, I'm really sorry that my criticism caused frustration and discouragement. I completely agree with everything you wrote. My criticism of the overuse of properties is more about the existing codebase and the need to change course rather than your contribution in particular. It was not so tactful for me to single out your code as an example of why property overuse leads to confusion. Especially since the previous review requested use of properties.

As you have realized, there are still a lot of structural issues in the codebase. A lot of things snuck in because I didn't review code as carefully as I should have. So it's a challenging situation. I want to be encouraging of contributions from people like you. But I also want the code to remain maintainable, and avoid further entrenching our previous bad design decisions.

You ask me to lay out a vision, but I think you already lay out a very clear vision in your last comment. The one thing I'd adjust is to avoid the idea of a monolithic printer object. I'd rather design things so that we have isolated stages, each of which only requires limited information. (Like the Linux philosophy of piping together simple commands.)

To figure out what these isolated stages should look like, let's suppose we were designing a web client as a replacement for the GUI. I think the first thing we'd need is a high-level specification of the label elements (e.g. text, barcode, etc.) that are to be horizontally combined. These shouldn't depend on the tape size or device, and it should ideally be easily serializable. Next would come the rasterization step that, for a given tape size in pixels, converts the elements into a bitmap to print. Finally, with the bitmap in hand, it should be passed to the printer for printing. So there are these steps: element specification, rasterization, printing to device, which I'd like to see as isolated as possible.

As for concrete next steps for this PR, let's try and make sure things are running without any errors. Let's try within reason to reduce the amount of stuff that's being added to interfaces and being passed around. And for these structural problems, let's keep track of what's wrong, and build in a way that will facilitate eventually fixing those structural problems.

And I'll try and to better to keep things fun. I think we're all here to build cool stuff in a relaxing environment. Sorry if I was being too up-tight. I really hope that you stick around.

[tomek-szczesny]

Since the project has a new name and a new scope (all label printers, ideally), it makes sense to define a sustainable architecture and overhaul it, thus bump the major release number. This might cost us less work in the long run, as with a few Dymos we already struggle to keep it tidy.

@maresb's general idea has some merit. I think that each printer model should be a different "object", with its own renderer code, attributes and so on.

One thing that worries me about this proposed approach is the second stage that composes the pieces together. I am thinking about barcodes and QR codes in particular, that will not work if scaled improperly. Thus this stage has to know how many dots it's working with, something that definitely is the domain of the third stage.

I guess we'll need a graph, and a space for this discussion :)

[maresb]

One thing that worries me about this proposed approach is the second stage that composes the pieces together. I am thinking about barcodes and QR codes in particular, that will not work if scaled improperly. Thus this stage has to know how many dots it's working with, something that definitely is the domain of the third stage.

Right, the pipeline can't be strictly one-way. In pseudocode I'd propose something like:

def print_label(label_specification, device, tape):
    bitmap_height = device.usable_tape_height_px(tape)
    bitmap = rasterize(label_specification, bitmap_height)
    device.print(bitmap, tape)

Of course it probably will end up more complicated than this in practice due to implementation details.

So for this PR I think my specific question would be if, instead of passing around the DymoLabeler object, would it be sufficient and feasible to compute the bitmap height upfront and pass around only that instead? My hope is that this would be able to considerably simplify the code in this PR, and make it easier in the future to refactor. (It may well be that I'm wrong and that would require rewriting the whole codebase, but I hope not! :see_no_evil:)

[FaBjE]

I suggested it, but I am not orthodox about it, and I do accept the reasoning raised earlier, as to why this was a bad suggestion. I am sorry for causing you to waste your eneregy, and lose some motivation to work on this codebase. I hope you'll continue your excellent work, and keep improving it. The architecture indeed needs some thoughts and design in order to make it better. I love the idea of separating contant printer data (print head specification, etc.) and runtime data (current tape loaded).

I would not call it a bad suggestion. Because every other thing is put into a property in the dymo labeler class. So in that way it made sense to me, and I changed it.

Hi @FaBjE, I'm really sorry that my criticism caused frustration and discouragement. I completely agree with everything you wrote. My criticism of the overuse of properties is more about the existing codebase and the need to change course rather than your contribution in particular. It was not so tactful for me to single out your code as an example of why property overuse leads to confusion. Especially since the previous review requested use of properties.

Well, you didn´t exactly put my code on the spot as the propery already existed. (I only renamed it). The confusing/frustrating part for me is that this PR is instead of discussing my changes is becoming more and more discussion about the global architecture of the project. Something I had nothing to do with, and should not overhaul in this PR. I did the best I could with the current codebase. But as stated before, most things I encountered would not be my first choice (either). But made sense if you would set this up for 1 printer type.

As you have realized, there are still a lot of structural issues in the codebase. A lot of things snuck in because I didn't review code as carefully as I should have. So it's a challenging situation. I want to be encouraging of contributions from people like you. But I also want the code to remain maintainable, and avoid further entrenching our previous bad design decisions.

Yes, but what you are currently doing now from my perspective is criticizing stuff out of scope of this PR.

You ask me to lay out a vision, but I think you already lay out a very clear vision in your last comment. The one thing I'd adjust is to avoid the idea of a monolithic printer object. I'd rather design things so that we have isolated stages, each of which only requires limited information. (Like the Linux philosophy of piping together simple commands.)

Well, yes, but that is my vision. I have no clue in where you are standing on this or what the goal of the project is.

My advice on your view would be that it might be the most ideal design. (keeping stages separate) But be realistic and keep it simple. This is at the moment a tiny project with about no-one with time to code on it. You will introduce a abstraction and challenges with it. Which all require effort to code.

To figure out what these isolated stages should look like, let's suppose we were designing a web client as a replacement for the GUI. I think the first thing we'd need is a high-level specification of the label elements (e.g. text, barcode, etc.) that are to be horizontally combined. These shouldn't depend on the tape size or device, and it should ideally be easily serializable. Next would come the rasterization step that, for a given tape size in pixels, converts the elements into a bitmap to print. Finally, with the bitmap in hand, it should be passed to the printer for printing. So there are these steps: element specification, rasterization, printing to device, which I'd like to see as isolated as possible.

Brutally honest, I (would like to) see labelle as the simple "simple but functional"/no-nonsense portable application that came on the Dymo PnP labeler (for windows) and I think you are 80% there atm. Thats what I am looking for at least. Just printing some labels from a GUI. nothing fancy.

An example that I read in the GUI issue is about the office setting. But not to ruin your dreams or anything. But the likely hood of something like labelle to be used in the foreseeable future are slim imho. For (professional) office settings there are much better (and expensive) solutions available.

I think labelle's strength will be in supporting the "older" hardware. and keeping it alive. reducing e-waste. Even on windows the official dymo software is pretty much out of support for these types of printers. But nothing is wrong with the label printers, I can buy tape cheaply at discount stores. and it can print labels fine.

Again, it may be fine ambitions, but something (too) far in the future imho. Especially considering the amount of active coders on the project.

On the pratical side as @tomek-szczesny pointed out in it's next post, the bitmap that needs to be rendered is direct dependent on the printer (dpi) and label (size).

As for concrete next steps for this PR, let's try and make sure things are running without any errors. Let's try within reason to reduce the amount of stuff that's being added to interfaces and being passed around. And for these structural problems, let's keep track of what's wrong, and build in a way that will facilitate eventually fixing those structural problems.

And I'll try and to better to keep things fun. I think we're all here to build cool stuff in a relaxing environment. Sorry if I was being too up-tight. I really hope that you stick around.

I have a different suggestion for this PR. I would like to see a "develop" branch. For this branch we use this PR as starting point.

A separate overhaul of the design may be implemented on top of that.

Maybe just in tiny steps to keep it manageable. (again, amount of available coders/time) Whenever we are happy with it, the develop branch can be merged to the main branch.

So for this PR I think my specific question would be if, instead of passing around the DymoLabeler object, would it be sufficient and feasible to compute the bitmap height upfront and pass around only that instead? My hope is that this would be able to considerably simplify the code in this PR, and make it easier in the future to refactor. (It may well be that I'm wrong and that would require rewriting the whole codebase, but I hope not! 🙈)

The dymo labeler object is current passed around for the DPI. Label height is already passed around. there are lots more things to consider if you want to support more devices. (for example, label width, all newer types have fixed size labels instead of our "endless" tape). But again, practically. This works.

So again my view. Merge this to develop, agree on a global design, rework everything (in tiny steps), have a proper codebase.

[maresb]

So again my view. Merge this to develop, agree on a global design, rework everything (in tiny steps), have a proper codebase.

I'd be happy to do this. That way we can follow up separately with the structural issues I've raised before merging to main, people can build on that if they'd like, and we avoid further blocking on this PR. Great suggestion!

I just work on this project to screw around. I have no serious ambitions. For quite a while I was the only active maintainer, and I was just trying to keep the project alive. I do have things I'd like to eventually see implemented, like a web client and server, but unfortunately I don't have the free time to implement that stuff myself.

Also, to be honest I don't understand the details of how all these classes are implemented, and whenever I look at the details I'm surprised and confused. It used to be way worse until @tomers came along and rewrote major portions of the codebase. He took us from 20% to 80% of having a stable functional app.