Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Device dependent margins/config #54

Merged
merged 27 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d73626d
Add "DYMO LabelMANAGER PC II" device ID
FaBjE Jun 15, 2024
06f61ea
Add 24mm tape type
FaBjE Jun 16, 2024
3aecdc7
device specific init WIP
FaBjE Jun 18, 2024
1872e40
Add DeviceConfig class for each printer
FaBjE Jun 20, 2024
1b279b3
Please mypy...
FaBjE Jun 20, 2024
2353512
Fix usb device recognition
FaBjE Jun 21, 2024
dcc6234
Calculate margins instead of using lookup table
FaBjE Jun 21, 2024
040f2c2
Force sending an "exact printhead width" bitmap to the printer
FaBjE Jun 21, 2024
f907b8b
Fix config not updated when device set
FaBjE Jun 21, 2024
d64c977
Tested on LabelManager PnP
FaBjE Jun 21, 2024
a7def5e
replace all camels with snakes
FaBjE Jun 22, 2024
d8dcb3d
Cleanup supported device list syntax
FaBjE Jun 23, 2024
4fc8b68
Convert device config to dataclass
FaBjE Jun 23, 2024
1d36049
Convert labeler device-config to property
FaBjE Jun 23, 2024
d8b3b6d
Move get_tape_print_size_and_margins_px to labeler class tape_print_p…
FaBjE Jun 23, 2024
13fa9b9
Revert GUI supported_tape_sizes type
FaBjE Jun 23, 2024
424ec34
Remove fixed default tapesize (take highest supported as default)
FaBjE Jun 23, 2024
d342a04
Shorten matches_device_id function
FaBjE Jun 23, 2024
1786928
Fix code convention in if statement
FaBjE Jun 23, 2024
16d82e8
Shorten if statement
FaBjE Jun 23, 2024
e239867
Shorten if statement
FaBjE Jun 23, 2024
b390217
Let get_device_config_by_id raise exception if not supported
FaBjE Jun 23, 2024
0bdf8fd
Remove var / shorten line
FaBjE Jun 23, 2024
a4ed930
fix comment
FaBjE Jun 23, 2024
fdf3538
Clear up comment block
FaBjE Jun 23, 2024
0fa7ad5
Fix wrong var name
FaBjE Jun 23, 2024
7246473
Remove constant DPI values and calculations
FaBjE Jun 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/labelle/gui/gui.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def _init_elements(self) -> None:
self._device_manager = DeviceManager()
self._dymo_labeler = DymoLabeler()
self._settings_toolbar.update_labeler_context(
supported_tape_sizes=self._dymo_labeler.SUPPORTED_TAPE_SIZES_MM,
supported_tape_sizes=self._dymo_labeler._deviceConfig.supportedTapeSizes,
maresb marked this conversation as resolved.
Show resolved Hide resolved
installed_tape_size=self._dymo_labeler.tape_size_mm,
minimum_horizontal_margin_mm=self._dymo_labeler.minimum_horizontal_margin_mm,
)
Expand Down
2 changes: 1 addition & 1 deletion src/labelle/gui/q_settings_toolbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def _init_elements(self) -> None:

def update_labeler_context(
self,
supported_tape_sizes: tuple[int, ...],
supported_tape_sizes: list[int],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really needed? In case the list is constant, it would be better as a convention to prefer tuple over list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The amount of supported tape sizes varies from printer to printer. I got an error that I tried to change the "tuple" size. A quick google search gave this as a solution. If there is a better solution please elaborate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, tuples are "immutable" so you should always generate a new one instead of modifying an old one.

Copy link
Contributor Author

@FaBjE FaBjE Jun 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed (It didn´t complain this time)

Edit:
Sorry, had to revert this:

src/labelle/gui/gui.py:66: error: Argument "supported_tape_sizes" to "update_labeler_context" of "QSettingsToolbar" has incompatible type "list[int]"; expected "tuple[int, ...]"  [arg-type]
Found 1 error in 1 file (checked 46 source files)

installed_tape_size: int,
minimum_horizontal_margin_mm: float,
) -> None:
Expand Down
149 changes: 135 additions & 14 deletions src/labelle/lib/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
# either sysfs is unavailable or unusable by this script for some reason.
# Please beware that DEV_NODE must be set to None when not used, else you will
# be bitten by the NameError exception.
from enum import Enum
from enum import Enum, IntEnum
from pathlib import Path

import labelle.resources.fonts
import labelle.resources.icons
from labelle.lib.devices.device_config import DeviceConfig

try:
from pyqrcode import QRCode
Expand All @@ -34,19 +35,139 @@
"WARNING: This device is not confirmed to work with this software. Please "
"report your experiences in https://github.com/labelle-org/labelle/issues/4"
)
SUPPORTED_PRODUCTS = {
0x0011: "DYMO LabelMANAGER PC",
0x0015: "LabelPoint 350",
0x1001: "LabelManager PnP (no mode switch)",
0x1002: "LabelManager PnP (mode switch)",
0x1003: f"LabelManager 420P (no mode switch) {UNCONFIRMED_MESSAGE}",
0x1004: f"LabelManager 420P (mode switch) {UNCONFIRMED_MESSAGE}",
0x1005: "LabelManager 280 (no mode switch)",
0x1006: "LabelManager 280 (no mode switch)",
0x1007: f"LabelManager Wireless PnP (no mode switch) {UNCONFIRMED_MESSAGE}",
0x1008: f"LabelManager Wireless PnP (mode switch) {UNCONFIRMED_MESSAGE}",
0x1009: f"MobileLabeler {UNCONFIRMED_MESSAGE}",
}


# Supported USB device ID enumeration
class SUPPORTED_DEVICE_ID(IntEnum):
LABELMANAGER_PC = 0x0011
LABELPOINT_350 = 0x0015
LABELMANAGER_PC_II = 0x001C
LABELMANAGER_PNP_NO_MODE_SWITCH = 0x1001
LABELMANAGER_PNP_MODE_SWITCH = 0x1002
LABELMANAGER_420P_NO_MODE_SWITCH = 0x1003
LABELMANAGER_420P_MODE_SWITCH = 0x1004
LABELMANAGER_280P_NO_MODE_SWITCH = 0x1005
LABELMANAGER_280P_MODE_SWITCH = 0x1006
LABELMANAGER_WIRELESS_PNP_NO_MODE_SWITCH = 0x1007
LABELMANAGER_WIRELESS_PNP_MODE_SWITCH = 0x1008
MOBILE_LABELER = 0x1009


# Create supported products list
SUPPORTED_PRODUCTS = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using append is a dynamic operation. I think it would look better to do

SUPPORTED_PRODUCTS = [
    DeviceConfig(
        ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try (the append was just copied from an example)


# ---- Supported USB Devices configuration ----
SUPPORTED_PRODUCTS.append(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that eventually we should move these out of the code, into a JSON file. If possible, please try to do it. In not, we can try refactor it later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a proper data model(1) for device models(2). (The first "model" in that sentence means "implementation of a schema" while the second "model" refers to a particular product that is sold by a company.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I was a bit confused You do already define DeviceConfig.

I'm wondering how to achieve this elegantly. I agree that it's in principle better to define these things out-of-code like in JSON. However, that unfortunately would require deserialization. Pydantic is a popular library for this, but then we're adding more project dependencies and a lot of complexity, also in the code.

The way I see it, we have two main options:

  1. Separate the device definition from code, perform deserialization, probably add a dependency on something like Pydantic.
  2. Keep the device definitions as code, but switch to NamedTuple or dataclass.

Another disadvantage to 1. is that editing the config would involve editing serialized data which can be a real pain. An advantage to 2. is that code tools like mypy will automatically perform validation for the data-as-code.

I'm leaning more towards 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I was trying to achieve here is a what in C would be "array of structs" for the "const" configuration.
If you have any suggestions how to do this elegantly in python, let me know...

My idea was that the (existing) labeler class took this as input to know the device's exact properties.

I agree that having this out of code would be nice. But can we please keep this contained?
I started out with "just adding my printer", and it is getting a bit out of hand already imho.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converted DeviceConfig to a dataclass

DeviceConfig(
"DYMO LabelMANAGER PC",
[int(SUPPORTED_DEVICE_ID.LABELMANAGER_PC)],
# ToDo: Validate config!
# Printhead 128 Pixels, distributed over 18mm of active area
128,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to understand from reading this what each field means. Please explicitly name each field.

Copy link
Contributor

@maresb maresb Jun 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be explicit, this is as simple as

DeviceConfig(
    name="DYMO LabelMANAGER PC",
    deviceIDs=[int(SUPPORTED_DEVICE_ID.LABELMANAGER_PC)],
    ...

Also, the use here of SUPPORTED_DEVICE_ID seems very indirect to me. Why not just do

    deviceIDs=[0x0011],

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that is possible can you give me a syntax example? (It would definitely be a lot clearer)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FaBjE that was an explicit example, and should work without other modifications to your code if you continue the obvious pattern. (Python lets you specify the names of positional arguments in the function invocation.) Does this make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn´t see your example at first. I had this page already open I guess.
But thanks! I will try to add.

I added the enum because before the DeviceConfig class I checked with if-else in the code which properties applied to the printer (Based on the device ID), but that was ugly imho.
I agree it is kind of redundant now. Shall I remove it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's an example:

def f(x, y):
    return 2 * x + y

f(y=1, x=2)  # 5

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

18,
[6, 9, 12, 19],
)
)
SUPPORTED_PRODUCTS.append(
DeviceConfig(
"LabelPoint 350",
[int(SUPPORTED_DEVICE_ID.LABELPOINT_350)],
# ToDo: Validate config!
# Printhead 64 Pixels, distributed over 9mm of active area
64,
9,
[6, 9, 12],
)
)
SUPPORTED_PRODUCTS.append(
DeviceConfig(
"DYMO LabelMANAGER PC II",
[int(SUPPORTED_DEVICE_ID.LABELMANAGER_PC_II)],
# Printhead 128 Pixels, distributed over 18mm of active area
128,
18,
[6, 9, 12, 19, 24],
)
)
SUPPORTED_PRODUCTS.append(
DeviceConfig(
"LabelManager PnP",
[
int(SUPPORTED_DEVICE_ID.LABELMANAGER_PNP_NO_MODE_SWITCH),
int(SUPPORTED_DEVICE_ID.LABELMANAGER_PNP_MODE_SWITCH),
],
# ToDo: Validate config!
# Printhead 64 Pixels, distributed over 9mm of active area
64,
9,
[6, 9, 12],
)
)
SUPPORTED_PRODUCTS.append(
DeviceConfig(
f"LabelManager 420P {UNCONFIRMED_MESSAGE}",
[
int(SUPPORTED_DEVICE_ID.LABELMANAGER_420P_NO_MODE_SWITCH),
int(SUPPORTED_DEVICE_ID.LABELMANAGER_420P_MODE_SWITCH),
],
# ToDo: Validate config!
# Printhead 64 Pixels, distributed over 9mm of active area
64,
9,
[6, 9, 12],
)
)
SUPPORTED_PRODUCTS.append(
DeviceConfig(
"LabelManager 280",
[
int(SUPPORTED_DEVICE_ID.LABELMANAGER_280P_MODE_SWITCH),
int(SUPPORTED_DEVICE_ID.LABELMANAGER_280P_NO_MODE_SWITCH),
],
# ToDo: Validate config!
# Printhead 64 Pixels, distributed over 9mm of active area
64,
9,
[6, 9, 12],
)
)
SUPPORTED_PRODUCTS.append(
DeviceConfig(
f"LabelManager Wireless PnP {UNCONFIRMED_MESSAGE}",
[
int(SUPPORTED_DEVICE_ID.LABELMANAGER_WIRELESS_PNP_NO_MODE_SWITCH),
int(SUPPORTED_DEVICE_ID.LABELMANAGER_WIRELESS_PNP_MODE_SWITCH),
],
# ToDo: Validate config!
# Printhead 64 Pixels, distributed over 9mm of active area
64,
9,
[6, 9, 12],
)
)
SUPPORTED_PRODUCTS.append(
DeviceConfig(
f"MobileLabeler {UNCONFIRMED_MESSAGE}",
[int(SUPPORTED_DEVICE_ID.MOBILE_LABELER)],
# ToDo: Validate config!
# Printhead 64 Pixels, distributed over 9mm of active area
64,
9,
[6, 9, 12],
)
)

# Simulator configuration
SIMULATOR_CONFIG = DeviceConfig(
"Simulator",
[0],
# Fake printhead 128 Pixels, distributed over 18mm of active area
128,
18,
[6, 9, 12, 19, 24],
)


DEV_VENDOR = 0x0922

PRINTER_INTERFACE_CLASS = 0x07
Expand Down
114 changes: 114 additions & 0 deletions src/labelle/lib/devices/device_config.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
from __future__ import annotations

import logging
import math

LOG = logging.getLogger(__name__)


class DeviceConfig:
"""Configuration object for the capabilities of a label printer."""

name: str
"""Name of this device"""

deviceIDs: list[int]
"""List of USB Device ID's this device can identify as"""

printHeadSizePixels: int
"""Size of the print head in pixels (use calibration routine to determine)"""

printHeadActiveAreaMillimeters: float
"""Size of the active area of the print head in millimters
(use calibration routine to determine)"""

supportedTapeSizes: list[int]
"""List of supported tape sizes in mm"""

# Fixed to 1 mm until proven otherwise ;)
TAPE_ALIGNMENT_INACCURACY_MM: float = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is float, please write 1.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

"""The inaccuracy of the tape position relative to the printhead"""
# Inaccuracy of the tape position is mostly caused by
# the tape moving slightly from side to side in the cartridge.
# Improper cartrigde placemement is also an factor,
# but negligible due to a physical spring in the lid.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make lines longer.
I think you should be able to write all comments under a single """ .... """ block, just make sure to add the extended description following a separating empty line. https://stackoverflow.com/a/23188939/1692287

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to make long lines, but there is a pre-commit hook from preventing me to...

I will add the extended description to the function comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    """The inaccuracy of the tape position relative to the printhead

    Inaccuracy of the tape position is mostly caused by
    the tape moving slightly from side to side in the cartridge.
    Improper cartrigde placemement is also an factor,
    but negligible due to a physical spring in the lid.
    """

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the pre-commit hook is not to make things pretty. Instead the idea is to reduce entropy, so that there are fewer ways to express equivalent code. (Even if it's uglier, at least it's more standardized.)

If it gets in your way, feel free to ignore the errors for now and use git commit --no-verify, and we can deal with it once at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed as suggested.


def __init__(
self,
name: str,
deviceIDs: list[int],
maresb marked this conversation as resolved.
Show resolved Hide resolved
printHeadSizePixels: int,
printHeadActiveAreaMillimeters: int,
supportedTapeSizes: list[int],
):
"""Initialize a Labeler config object."""
self.name = name
self.deviceIDs = deviceIDs
self.printHeadSizePixels = printHeadSizePixels
self.printHeadActiveAreaMillimeters = printHeadActiveAreaMillimeters
self.supportedTapeSizes = supportedTapeSizes

def matches_device_id(self, idValue: int) -> bool:
"""Check if the a device ID matches this config."""
if idValue in self.deviceIDs:
return True
else:
return False
Copy link
Contributor

@tomers tomers Jun 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure a function is really needed for this, as it is rather short, and can be replaced with a simple expression. In any case, it should be refactored as follows:

return idValue in self.deviceIDs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix. It seemed more "contained" to me than just accessing the array itself from outside the object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


def get_tape_print_size_and_margins_px(
Copy link
Contributor

@tomers tomers Jun 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this a property, rather a get_ function.

It should be:

@propery
def active_tape_pixels(self) -> int
    ...

And then there will be another derived propery:

@propery
def top_margin(self) -> int
    return round(((self.printHeadSizePixels - self.active_tape_pixels) / 2), 0)

I don't think you use bottom_margin, but in case you do:

@propery
def bottom_margin(self) -> int
    return self.top_margin

Actually, if you do use bottom_margin, maybe just called it vertical_print_margin or something like that, and use the same margin both for top and bottom.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The margins depend on the tape size selected as input.
A property that has a value based on another property's value seems very dirty to me.

I kept the bottom margin for the sake of completeness, they are equal now due to the calculation but that might not always be the case. The current rendering class only uses the top-margin but that could change too...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the labeler class and made it a property / named tuple

self, tapeSizeMM: int
) -> tuple[int, int, int]:
"""Get print margins for a specific tape size.

:param tapeSizeMM: Tape size in mm
:return: Margins tuple in pixels [ActivePixels,TopMarginPx,BottomMarginPx]
"""
if tapeSizeMM in self.supportedTapeSizes:
# Calculate the pixels per mm for this printer
# Example: printhead of 128 Pixels, distributed over 18 mm of active area.
# Makes 7.11 pixels/mm
printPPmm: float = (
self.printHeadSizePixels / self.printHeadActiveAreaMillimeters
)

# Calculate usable tape width (*2 for top and bottom)
usableTapeWidth: float = tapeSizeMM - (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this a property, not a calculated variable inside a function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is that you need the tape with as input to calculate this.
am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the labeler class and made it a property

self.TAPE_ALIGNMENT_INACCURACY_MM * 2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is a really silly comment, but since in my mind I read "tape width minus twice the margins", I would write 2 * self.TAPE_ALIGNMENT_INACCURACY_MM instead of self.TAPE_ALIGNMENT_INACCURACY_MM * 2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If It makes you happy, I'll change it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

)

# Calculate the numer of active pixels for the tape
activeTapePixels: float = 0
if usableTapeWidth >= self.printHeadActiveAreaMillimeters:
# Tape is larger than active area of printhead. Use all pixels
activeTapePixels = self.printHeadSizePixels
else:
# Calculate the amount of active pixels we are able to use
# (taking the placement inaccuracy into account)
activeTapePixels = printPPmm * usableTapeWidth

# Round down to nearest whole number as we can't use half a pixels ;)
activeTapePixels = math.floor(activeTapePixels)

# To calculate the margins we need to know some hardware info
# Printer has special "support studs" that
# let 19 & 24mm tapes go to the bottom (if supported)
# but 12mm based casettes are raised so they are centered to the printhead.
# Smaller tapes than 12mm are in 12mm casettes are centered
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some rephrasing is needed here: "are in 12mm casettes are centered"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats better wording indeed. will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded / made it better aligned

# in the cartridge and in turn also centered to the printhead
# Which gives us the advantage that we can
# just calculate the top and bottom margin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please break lines according to our linter line width (120 chars). Having too short lines makes it harder to read.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it is 88 chars

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the result of the pre-commit hook that is configured. Maybe something is wrong on my end but I was not allowed to commit with these "long lines"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworded / made it better aligned


# Calculate the top margin
topMargin = round(((self.printHeadSizePixels - activeTapePixels) / 2), 0)

# Bottom margin is equal due to centering of the tape
bottomMargin = topMargin

# Return active pixels / margins set
return (int(activeTapePixels), int(topMargin), int(bottomMargin))
else:
# Tape size not supported
raise ValueError(
f"Unsupported tape size {tapeSizeMM}mm. "
f"Supported sizes: {self.supportedTapeSizes}mm"
)
22 changes: 21 additions & 1 deletion src/labelle/lib/devices/device_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
SUPPORTED_PRODUCTS,
UNCONFIRMED_MESSAGE,
)
from labelle.lib.devices.device_config import DeviceConfig
from labelle.lib.devices.usb_device import UsbDevice

LOG = logging.getLogger(__name__)
Expand Down Expand Up @@ -80,8 +81,27 @@ def find_and_select_device(self, patterns: list[str] | None = None) -> UsbDevice
LOG.debug(dev.device_info)
dev = devices[0]
if dev.is_supported:
msg = f"Recognized device as {SUPPORTED_PRODUCTS[dev.id_product]}"
foundDeviceConfig: DeviceConfig | None = get_device_config_by_id(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need to declare the type of foundDeviceConfig (I guess you meant found_device_config :-) )?
If the function declares its return type, then I think it should be fine not to declare here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this was an ugly workaround. Will be obsolete when I fix the other function.
And yes of course I meant snake_case 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

dev.id_product
)
if foundDeviceConfig is not None:
msg = f"Recognized device as {foundDeviceConfig.name}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is msg if found_device_config is None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure how to proceed than. Just print nothing?
Is there a quick syntax to "add if not null, otherwise leave empty"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

else:
msg = f"Unrecognized device: {hex(dev.id_product)}. {UNCONFIRMED_MESSAGE}"
LOG.debug(msg)
return dev


def get_device_config_by_id(idValue: int) -> DeviceConfig | None:
"""Get a labeler device config with USB ID.

:param idValue: USB ID value
:return: Device config, None if not found
"""
#
for device in SUPPORTED_PRODUCTS:
if device.matches_device_id(idValue) is True:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove is True. It will work the same

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

return device

# No device config found
return None
Loading