Skip to content

Commit 137c1a2

Browse files
authored
Refactor: modernize helper_methods + port Mainsail layer formulas to jobStatusPage (#208)
# Description - [ ] Feature - [ ] Bug fix - [x] Code refactor - [ ] Documentation Refactors `helper_methods.py` and `jobStatusPage.py`, and adds a full unit test suite for `JobStatusWidget`. **jobStatusPage.py** - `_VALID_STATES` / `_INVALID_STATES` -> `frozenset` class constants (avoid per-call realloc) - `on_gcode_move_update` - 7 individual guards collapsed into one compound `or` guard - `on_print_stats_update` - `if/if` -> `if/elif` chains (mutually exclusive branches) - `_post_event` helper - DRY up duplicated `QApplication.postEvent` pattern - `_load_thumbnails` - manual loop -> walrus list comprehension - Removed no-op `QPixmap.scaled()` to identical dimensions - Removed spurious `self.show()` on thumbnail collapse **helper_methods.py** - Mainsail layer formula extracted to `calculate_current_layer` / `calculate_max_layers` - jobStatusPage updated to call the new helpers **tests/widgets/test_job_status_page_unit.py** - 40 unit tests - `TestOnPrintStart` (4), `TestHandlePrintState` (7), `TestOnPrintStatsUpdate` (6) - `TestOnGcodeMoveUpdate` (8), `TestVirtualSdcardUpdate` (3) - `TestPauseResumePrint` (6), `TestHandleCancel` (4), `TestOnFileInfo` (5) **tests/widgets/conftest.py** - Ensures `BlocksScreen/` on `sys.path` for `lib.*` imports - Clears empty `lib.*` namespace stubs registered by the network conftest so widget tests can coexist in the full `make test-fast` run # Motivation The refactors reduce duplicated code and allocation overhead in the hottest print-progress paths. The test suite documents the expected slot behaviour and will catch regressions in the state machine, signal wiring, and layer-calculation logic. # Tests Widget tests alone: `40 passed`
1 parent 560d8de commit 137c1a2

4 files changed

Lines changed: 583 additions & 232 deletions

File tree

‎BlocksScreen/helper_methods.py‎

Lines changed: 81 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,10 @@
99
import ctypes
1010
import enum
1111
import logging
12+
import math
1213
import os
1314
import pathlib
1415
import struct
15-
import typing
1616

1717
logger = logging.getLogger(__name__)
1818

@@ -21,7 +21,7 @@
2121
libxext = ctypes.CDLL("libXext.so.6")
2222

2323
class DPMSState(enum.Enum):
24-
"""Available DPMS states"""
24+
"""Available DPMS states."""
2525

2626
FAIL = -1
2727
ON = 0
@@ -86,7 +86,7 @@ def set_dpms_mode(mode: DPMSState) -> None:
8686
finally:
8787
libxext.XCloseDisplay(display)
8888

89-
def get_dpms_timeouts() -> typing.Dict:
89+
def get_dpms_timeouts() -> dict:
9090
"""Get current DPMS timeouts"""
9191
_display_name = ctypes.c_char_p(b":0")
9292
libxext.XOpenDisplay.restype = ctypes.c_void_p
@@ -118,9 +118,7 @@ def get_dpms_timeouts() -> typing.Dict:
118118
"off_seconds": _off_timeout,
119119
}
120120

121-
def set_dpms_timeouts(
122-
suspend: int = 0, standby: int = 0, off: int = 0
123-
) -> typing.Dict:
121+
def set_dpms_timeouts(suspend: int = 0, standby: int = 0, off: int = 0) -> dict:
124122
"""Set DPMS timeout"""
125123
_display_name = ctypes.c_char_p(b":0")
126124
libxext.XOpenDisplay.restype = ctypes.c_void_p
@@ -155,11 +153,11 @@ def set_dpms_timeouts(
155153
"off_seconds": _off_timeout,
156154
}
157155

158-
def get_dpms_info() -> typing.Dict:
156+
def get_dpms_info() -> dict:
159157
"""Get DPMS information
160158
161159
Returns:
162-
typing.Dict: Dpms state
160+
dict: Dpms state
163161
"""
164162
_dpms_state = DPMSState.FAIL
165163
onoff = 0
@@ -227,133 +225,122 @@ def disable_dpms() -> None:
227225
logger.exception(f"Unexpected exception occurred {e}")
228226

229227

230-
def convert_bytes_to_mb(self, bytes: int | float) -> float:
231-
"""Converts byte size to megabyte size
228+
def convert_bytes_to_mb(size_bytes: int | float) -> float:
229+
"""Converts byte size to megabyte size.
232230
233231
Args:
234-
bytes (int | float): bytes
232+
size_bytes: Value in bytes.
235233
236234
Returns:
237-
mb: float that represents the number of mb
235+
Equivalent value in megabytes.
238236
"""
239237
_relation = 2 ** (-20)
240-
return bytes * _relation
238+
return size_bytes * _relation
241239

242240

243241
def calculate_current_layer(
244242
z_position: float,
245-
object_height: float,
246243
layer_height: float,
247244
first_layer_height: float,
245+
max_layers: int = 0,
248246
) -> int:
249-
"""Calculated the current printing layer given the GCODE z position received by the
250-
gcode_move object update.
251-
Also updates the label where the current layer should be displayed
247+
"""Calculate current layer from Z position (fallback when Klipper
248+
does not provide ``print_stats.info.current_layer``).
249+
250+
Formula ported from Mainsail ``getPrintCurrentLayer`` getter:
251+
``src/store/printer/getters.ts`` in ``mainsail-crew/mainsail``.
252+
253+
Uses ``ceil((z - first_layer_height) / layer_height + 1)``
254+
and clamps the result to ``[0, max_layers]``.
252255
253256
Returns:
254-
int: Current layer
257+
int: Current layer number (0 when not yet printing).
255258
"""
256-
if z_position == 0:
257-
return -1
258-
if z_position <= first_layer_height:
259-
return 1
259+
if layer_height <= 0 or first_layer_height < 0:
260+
return 0
260261

261-
_current_layer = (z_position) / layer_height
262+
layer = math.ceil((z_position - first_layer_height) / layer_height + 1)
263+
if max_layers > 0 and layer > max_layers:
264+
return max_layers
265+
return layer if layer > 0 else 0
262266

263-
return int(_current_layer)
264267

268+
def calculate_max_layers(
269+
object_height: float,
270+
layer_height: float,
271+
first_layer_height: float,
272+
) -> int:
273+
"""Calculate total layers from metadata dimensions (fallback when
274+
Klipper does not provide ``print_stats.info.total_layer``).
265275
266-
def estimate_print_time(seconds: int) -> list:
267-
"""Convert time in seconds format to days, hours, minutes, seconds.
276+
Formula ported from Mainsail ``getPrintMaxLayers`` getter:
277+
``src/store/printer/getters.ts`` in ``mainsail-crew/mainsail``.
268278
269-
Args:
270-
seconds (int): Seconds
279+
Uses ``ceil((object_height - first_layer_height) / layer_height + 1)``.
271280
272281
Returns:
273-
list: list that contains the converted information [days, hours, minutes, seconds]
282+
int: Total layer count, or 0 if metadata is insufficient.
274283
"""
275-
num_min, seconds = divmod(seconds, 60)
276-
num_hours, minutes = divmod(num_min, 60)
277-
days, hours = divmod(num_hours, 24)
278-
return [days, hours, minutes, seconds]
279-
284+
if layer_height <= 0 or object_height <= 0:
285+
return 0
286+
return max(1, math.ceil((object_height - first_layer_height) / layer_height + 1))
280287

281-
def normalize(value, r_min=0.0, r_max=1.0, t_min=0.0, t_max=100):
282-
"""Normalize values between a rage"""
283-
# https://stats.stackexchange.com/questions/281162/scale-a-number-between-a-range
284-
c1 = (value - r_min) / (r_max - r_min)
285-
c2 = (t_max - t_min) + t_min
286-
return c1 * c2
287288

289+
def estimate_print_time(seconds: int) -> list[int]:
290+
"""Convert *seconds* to ``[days, hours, minutes, seconds]``."""
291+
num_min, secs = divmod(seconds, 60)
292+
num_hours, mins = divmod(num_min, 60)
293+
days, hours = divmod(num_hours, 24)
294+
return [days, hours, mins, secs]
288295

289-
def check_filepath_permission(filepath, access_type: int = os.R_OK) -> bool:
290-
# if not isinstance(filepath, pathlib.Path):
291-
"""Checks for file path access
292296

293-
Args:
294-
filepath (str | pathlib.Path): path to file
295-
access_type (int, optional): _description_. Defaults to os.R_OK.
297+
def normalize(
298+
value: float,
299+
r_min: float = 0.0,
300+
r_max: float = 1.0,
301+
t_min: float = 0.0,
302+
t_max: float = 100.0,
303+
) -> float:
304+
"""Scale *value* from range [r_min, r_max] into [t_min, t_max]."""
305+
return (value - r_min) / (r_max - r_min) * (t_max - t_min) + t_min
296306

297-
***
298307

299-
#### **Access type can be:**
308+
def check_filepath_permission(
309+
filepath: str | pathlib.Path, access_type: int = os.R_OK
310+
) -> bool:
311+
"""Check whether *filepath* exists and has the requested access.
300312
301-
- F_OK -> Checks file existence on path
302-
- R_OK -> Checks if file is readable
303-
- W_OK -> Checks if file is Writable
304-
- X_OK -> Checks if file can be executed
313+
Args:
314+
filepath: Path to file.
315+
access_type: ``os.F_OK`` (existence), ``os.R_OK`` (read),
316+
``os.W_OK`` (write), or ``os.X_OK`` (execute).
305317
306-
***
307318
Returns:
308-
bool: _description_
309-
""" # return False
310-
if not os.path.isfile(filepath):
311-
return False
312-
return os.access(filepath, access_type)
319+
``True`` if the file exists and satisfies *access_type*.
320+
"""
321+
path = pathlib.Path(filepath)
322+
return path.is_file() and os.access(path, access_type)
313323

314324

315-
def check_dir_existence(
316-
directory: typing.Union[str, pathlib.Path],
317-
) -> bool:
318-
"""Check if a directory exists. Returns a true if it exists"""
319-
if isinstance(directory, pathlib.Path):
320-
return bool(directory.is_dir())
321-
return bool(os.path.isdir(directory))
325+
def check_dir_existence(directory: str | pathlib.Path) -> bool:
326+
"""Return ``True`` if *directory* exists and is a directory."""
327+
return pathlib.Path(directory).is_dir()
322328

323329

324330
def check_file_on_path(
325-
path: typing.Union[typing.LiteralString, pathlib.Path],
326-
filename: typing.Union[typing.LiteralString, pathlib.Path],
331+
path: str | pathlib.Path,
332+
filename: str | pathlib.Path,
327333
) -> bool:
328-
"""Check if file exists on path. Returns true if file exists on that specified directory"""
329-
_filepath = os.path.join(path, filename)
330-
return os.path.exists(_filepath)
331-
334+
"""Return ``True`` if *filename* exists under *path*."""
335+
return (pathlib.Path(path) / filename).exists()
332336

333-
def get_file_loc(filename) -> pathlib.Path: ...
334337

338+
def get_file_name(filename: str | None) -> str:
339+
"""Extract the basename from a file path (handles ``/`` and ``\\``).
335340
336-
def get_file_name(filename: typing.Optional[str]) -> str:
337-
# If filename is None or empty, return empty string instead of None
341+
Returns:
342+
The last path component, or ``""`` if *filename* is falsy.
343+
"""
338344
if not filename:
339345
return ""
340-
# Remove trailing slashes or backslashes
341-
filename = filename.rstrip("/\\")
342-
343-
# Normalize Windows backslashes to forward slashes
344-
filename = filename.replace("\\", "/")
345-
346-
parts = filename.split("/")
347-
348-
# Split and return the last path component
349-
return parts[-1] if filename else ""
350-
351-
352-
# def get_hash(data) -> hashlib._Hash:
353-
# hash = hashlib.sha256()
354-
# hash.update(data.encode())
355-
# hash.digest()
356-
# return hash
357-
358-
359-
def digest_hash() -> None: ...
346+
return pathlib.PurePosixPath(filename.replace("\\", "/")).name

0 commit comments

Comments
 (0)