Beacon/first layer improvements #101
Conversation
…ints, not the interpolated points.
…E_BEACON_COMPENSATION_MESH
…can compensation mesh issues rather than failing late eg after heat soaking.
…reasonably close to the current beacon coil temp. Currently allowing up to 20C difference as the acceptable range.
…ault name for the beacon scan compensation mesh
…n model according to beacon_contact_calibrate_model_on_print
…ION, now always saves. From discussion with MS there was no known use case for *not* saving, and not saving creates a profile in a confusing ephemeral state which is lost when the profile is reloaded.
- Implement _RAISE_ERROR in python instead of jinja for cleaner error messages - Implement _TRY, the try/except/finally pattern for macros
…tatus instead of raising an error.
…ated functionality
- Change and enhance the set of metadata stored:
- Remove beacon_model_temp as this is not relevant.
- Add bed temperature, mesh kind and probe method.
- Add warnings in create_compensation_mesh() for unapplied z-tilt or QGL
- Add various new gcode commands
- Defer profile deserialization using delayed gcode to ensure that warnings are seen in the console
- Replace/improve checks and warnings/errors
- Enhance preflight checks called from START_PRINT
- Apply extended metadata to all profiles/meshes created via BED_MESH_CALIBRATE
- Support saving via the standard Z_OFFSET_APPLY_PROBE macro - Exclude active IDEX toolhead offset from the saved value
…tions - TEMPERATURE_WAIT now calls TEMPERATURE_WAIT_BASE even when both MINIMUM and MAXIMUM are not specified, allowing the base impl to raise an error
- this protects against brief transitions when the count of active steppers briefly hits zero, and allows any final time-sensitive tasks to complete when motors are turned off. - also update some nomenclature for clarity
…_RATOS_CALIBRATE - Scan compensation mesh creation is always performed if beacon_scan_compensation_enable is true, we don't use the gantry twist check any more. From extensive testing, every printer tested benefits from a compensation mesh, with at least 70um range of correction, typically over 120um. Further, the twist check is not reliable due to the micro-positional variation of beacon contact readings and the low spatial resolution of the twist check not reliably capturing significant details. - Scan compensation mesh creation is now performed before BEACON_FINAL_CALIBRATION, as comp mesh creation requires a more thorough heat soak than BEACON_FINAL_CALIBRATION.
…rors and warnings logged via console_echo so they are less-easily missed
…napplied Z-tilt or quad gantry leveling - this behaviour can be overriden using [gcode_macro BED_MESH_CALIBRATE] variable_allow_unapplied_abl: True - combine beacon and stowable BED_MESH_CALIBRATE overrides, relocate to overrides.cfg - fix spelling of "leveling" in a few places
…or V-Core 4 variants - Aspects of the adapative heatsoak algorithm are currently tuned to the V-Core 4 design
…n for now, this needs further consideration (partial revert of 6b74598) - the previous behaviour breaks the expected story of the Mainsail heightmap page "click home icon, click calibrate"
… hotend_heat_soak_time
…safe home position
- leave RATOS_INIT initial_duration as 0.1. Among other things, this macro calculates the printable area which must happen before other init code. - change other init macros that were using 0.1 to 0.2, with the aim of ensuring that RATOS_INIT has been run first.
…d rehome Z if it has not been applied
- no specific hazard was identified, but in case future code leads to a pre-init call, there will now be a clear error rather than weird behaviour.
- This fixes an issue that occurred after the last toolshift, where the expansion offset for the now-deactivated toolhead heater being set to zero would be applied to the remaining active toolhead, creating an oversquished layer.
…rrectly in `START_PRINT` - This fixes an issue where under-temperature extrusion is attempted on the first toolchange because the inactive toolhead is at standby temperature but is not flagged as being in standby, so does not get heated to printing temperature.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces significant enhancements and refactoring across the RatOS configuration, macros, and Klipper extension modules. Key changes include new Klipper extensions for adaptive beacon heat soak, true zero correction, dynamic CPU governor, and fast configuration access. Macro logic is modernized to use fast configuration, with expanded beacon features, improved mesh handling, robust error checking, and new debugging tools. Numerous configuration and macro files are updated for consistency and expanded functionality, especially for beacon-enabled workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Klipper
participant RatOS
participant Beacon
participant BedMesh
participant FastConfig
participant DynamicGovernor
User->>Klipper: Start Print / Home / Calibrate
Klipper->>RatOS: Macro/Command Invocation
RatOS->>FastConfig: Fetch immutable config snapshot
RatOS->>Beacon: Initiate beacon operation (e.g. heat soak, true zero)
Beacon->>Klipper: Probe/Calibrate/Monitor Z
RatOS->>BedMesh: Mesh calibration/compensation
RatOS->>DynamicGovernor: Switch CPU governor (on stepper activity)
Klipper->>User: Status/Progress/Error Reporting
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Complexity: This update introduces multiple new extension modules, major macro and configuration refactoring, beacon feature expansion, error handling improvements, and deep integration across macros, Python extensions, and configuration. The review will require careful attention to new control flows, error paths, and compatibility with existing printer setups. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (9)
configuration/printers/base.cfg (1)
45-47: Fix typo in comment for clarityMinor spelling slip (“arfetacts”) → “artifacts”.
-split_delta_z: 0.01 # Avoid visible surface stripe arfetacts +split_delta_z: 0.01 # Avoid visible surface stripe artifactsconfiguration/klippy/beacon_true_zero_correction.py (1)
242-251: Consider caching nozzle diameter for IDEX configurations.In IDEX mode, the nozzle diameter lookup happens for each call. Consider caching or passing the active extruder to avoid repeated lookups.
- def _get_nozzle_diameter(self): + def _get_nozzle_diameter(self, extruder_name=None): - extruder_name = 'extruder' + if extruder_name is None: + extruder_name = 'extruder' - if self.dual_carriage and self.dual_carriage.dc[1].mode.lower() == 'primary': - extruder_name = 'extruder1' + if self.dual_carriage and self.dual_carriage.dc[1].mode.lower() == 'primary': + extruder_name = 'extruder1'configuration/klippy/beacon_adaptive_heat_soak.py (1)
73-75: Define magic number as a constantThe minimum threshold value of 10.0 should be defined as a class constant for better maintainability.
class ThresholdPredictor: + MIN_THRESHOLD = 10.0 # Below this, measurements approach noise floor def __init__(self, printer):And update the usage:
- # below this number approach the noise floor of the system and are not useful. - t = max(t, 10.0) + # below this number approach the noise floor of the system and are not useful. + t = max(t, self.MIN_THRESHOLD)configuration/klippy/ratos.py (2)
644-648: Simplify nested if statementsThe static analyzer correctly identifies that these nested if statements can be combined.
- if safe_home_x < safe_min_x or safe_home_x > safe_max_x or safe_home_y < safe_min_y or safe_home_y > safe_max_y: - if not self.printer.is_shutdown(): - logging.info(f"{self.name}: (safe_home_x, safe_home_y) is not within beacon-probable region: printable_x_max={printable_x_max}, printable_y_max={printable_y_max}, safe_home_x={safe_home_x}, safe_home_y={safe_home_y}, raw_safe_home_x={raw_safe_home_x}, raw_safe_home_y={raw_safe_home_y}, beacon probing region: ({safe_min_x}, {safe_min_y}) - ({safe_max_x}, {safe_max_y})") - self.printer.invoke_shutdown(f"{self.name}: (safe_home_x, safe_home_y) must be within the region that Beacon can probe: ({safe_min_x}, {safe_min_y}) - ({safe_max_x}, {safe_max_y}). The configured location ({safe_home_x:.2f}, {safe_home_y:.2f}) is outside this region.") + if (safe_home_x < safe_min_x or safe_home_x > safe_max_x or safe_home_y < safe_min_y or safe_home_y > safe_max_y) and not self.printer.is_shutdown(): + logging.info(f"{self.name}: (safe_home_x, safe_home_y) is not within beacon-probable region: printable_x_max={printable_x_max}, printable_y_max={printable_y_max}, safe_home_x={safe_home_x}, safe_home_y={safe_home_y}, raw_safe_home_x={raw_safe_home_x}, raw_safe_home_y={raw_safe_home_y}, beacon probing region: ({safe_min_x}, {safe_min_y}) - ({safe_max_x}, {safe_max_y})") + self.printer.invoke_shutdown(f"{self.name}: (safe_home_x, safe_home_y) must be within the region that Beacon can probe: ({safe_min_x}, {safe_min_y}) - ({safe_max_x}, {safe_max_y}). The configured location ({safe_home_x:.2f}, {safe_home_y:.2f}) is outside this region.")
761-761: Simplify dictionary key checkUse
key in dictinstead ofkey in dict.keys()for better performance and readability.- return [(name, locals.get(name,None)) for name in signature.parameters.keys() if type(locals.get(name, None)).__name__ == type_name] + return [(name, locals.get(name,None)) for name in signature.parameters if type(locals.get(name, None)).__name__ == type_name]configuration/klippy/beacon_mesh.py (3)
1-1: Fix typo in module header comment-# Beacaon contact compensation mesh +# Beacon contact compensation mesh
172-173: Fix typo in comment-# run klippers inompatible profile check which is never called by bed_mesh +# run klipper's incompatible profile check which is never called by bed_mesh
872-872: Simplify dictionary iteration-for profile_name in profiles.keys(): +for profile_name in profiles:configuration/z-probe/beacon.cfg (1)
19-19: Fix typo in comment-# Using more than one mesh run is not currently reccomended because the Beacon extension determines the +# Using more than one mesh run is not currently recommended because the Beacon extension determines the
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
configuration/klippy/beacon_adaptive_heat_soak_model_training.csvis excluded by!**/*.csv
📒 Files selected for processing (45)
configuration/homing.cfg(8 hunks)configuration/klippy/beacon_adaptive_heat_soak.py(1 hunks)configuration/klippy/beacon_mesh.py(3 hunks)configuration/klippy/beacon_true_zero_correction.py(1 hunks)configuration/klippy/dynamic_governor.py(1 hunks)configuration/klippy/fastconfig.py(1 hunks)configuration/klippy/ratos.py(10 hunks)configuration/klippy/ratos_z_offset.py(1 hunks)configuration/macros.cfg(20 hunks)configuration/macros/idex/idex_is.cfg(2 hunks)configuration/macros/idex/toolheads.cfg(6 hunks)configuration/macros/idex/util.cfg(2 hunks)configuration/macros/idex/vaoc.cfg(5 hunks)configuration/macros/mesh.cfg(9 hunks)configuration/macros/overrides.cfg(8 hunks)configuration/macros/parking.cfg(1 hunks)configuration/macros/priming.cfg(7 hunks)configuration/macros/util.cfg(3 hunks)configuration/moonraker.conf(1 hunks)configuration/printers/base.cfg(1 hunks)configuration/printers/v-core-3-hybrid/macros.cfg(1 hunks)configuration/printers/v-core-3-idex/macros.cfg(2 hunks)configuration/printers/v-core-3/macros.cfg(1 hunks)configuration/printers/v-core-4-hybrid/300.cfg(0 hunks)configuration/printers/v-core-4-hybrid/400.cfg(0 hunks)configuration/printers/v-core-4-hybrid/500.cfg(0 hunks)configuration/printers/v-core-4-hybrid/macros.cfg(2 hunks)configuration/printers/v-core-4-hybrid/v-core-4-hybrid.cfg(1 hunks)configuration/printers/v-core-4-idex/300.cfg(0 hunks)configuration/printers/v-core-4-idex/400.cfg(0 hunks)configuration/printers/v-core-4-idex/500.cfg(0 hunks)configuration/printers/v-core-4-idex/macros.cfg(2 hunks)configuration/printers/v-core-4-idex/v-core-4-idex.cfg(1 hunks)configuration/printers/v-core-4/300.cfg(0 hunks)configuration/printers/v-core-4/400.cfg(0 hunks)configuration/printers/v-core-4/500.cfg(0 hunks)configuration/printers/v-core-4/macros.cfg(1 hunks)configuration/printers/v-core-4/v-core-4.cfg(1 hunks)configuration/printers/v-core-pro/macros.cfg(1 hunks)configuration/scripts/ratos-common.sh(1 hunks)configuration/scripts/ratos-install.sh(1 hunks)configuration/z-probe/beacon.cfg(18 hunks)configuration/z-probe/stowable-probe.cfg(0 hunks)src/scripts/common.sh(1 hunks)src/server/gcode-processor/Actions.ts(1 hunks)
💤 Files with no reviewable changes (10)
- configuration/printers/v-core-4-hybrid/400.cfg
- configuration/printers/v-core-4/500.cfg
- configuration/printers/v-core-4/300.cfg
- configuration/printers/v-core-4-hybrid/300.cfg
- configuration/printers/v-core-4-hybrid/500.cfg
- configuration/printers/v-core-4-idex/400.cfg
- configuration/z-probe/stowable-probe.cfg
- configuration/printers/v-core-4-idex/300.cfg
- configuration/printers/v-core-4-idex/500.cfg
- configuration/printers/v-core-4/400.cfg
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: LucidityCrash
PR: Rat-OS/RatOS-configurator#89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in `configuration/printers/v-core-pro/` in the source repository, but are accessed via `RatOS/printers/v-core-pro/` paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/printers/v-core-4-idex/v-core-4-idex.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/scripts/ratos-common.sh (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/printers/v-core-3/macros.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/printers/v-core-4-hybrid/v-core-4-hybrid.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/macros/util.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/homing.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/printers/v-core-4/v-core-4.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/macros/idex/idex_is.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/printers/v-core-pro/macros.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
configuration/printers/v-core-4/macros.cfg (1)
Learnt from: LucidityCrash
PR: #89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in configuration/printers/v-core-pro/ in the source repository, but are accessed via RatOS/printers/v-core-pro/ paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
🧬 Code Graph Analysis (2)
src/server/gcode-processor/Actions.ts (1)
src/server/gcode-processor/errors.ts (1)
SlicerNotSupported(36-36)
configuration/klippy/beacon_mesh.py (2)
configuration/klippy/fastconfig.py (3)
items(76-77)get(67-68)keys(70-71)configuration/klippy/ratos.py (3)
register_commands(109-123)debug_echo(543-544)console_echo(546-572)
🪛 Ruff (0.12.2)
configuration/klippy/dynamic_governor.py
86-89: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
91-94: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
96-99: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
118-121: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
123-126: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
128-131: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
configuration/klippy/ratos.py
644-645: Use a single if statement instead of nested if statements
(SIM102)
761-761: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
configuration/klippy/beacon_true_zero_correction.py
86-86: Comparison to None should be cond is None
Replace with cond is None
(E711)
207-207: f-string without any placeholders
Remove extraneous f prefix
(F541)
393-393: f-string without any placeholders
Remove extraneous f prefix
(F541)
configuration/klippy/beacon_mesh.py
294-295: Use a single if statement instead of nested if statements
(SIM102)
662-665: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
792-792: Local variable eventtime is assigned to but never used
Remove assignment to unused variable eventtime
(F841)
795-795: Test for membership should be not in
Convert to not in
(E713)
803-803: Local variable debug_lines is assigned to but never used
Remove assignment to unused variable debug_lines
(F841)
872-872: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
configuration/klippy/beacon_adaptive_heat_soak.py
381-381: f-string without any placeholders
Remove extraneous f prefix
(F541)
520-520: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
641-641: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
692-692: Function definition does not bind loop variable samples
(B023)
🔇 Additional comments (70)
configuration/printers/base.cfg (1)
48-50: Empty sections verified unique; confirm module defaultsA quick grep shows
[fastconfig]and[dynamic_governor]only appear inconfiguration/printers/base.cfg(lines 48 and 50). Before we merge, please verify that both Python modules tolerate an empty or missing config dict (i.e., they don’t require any mandatory keys). If they do require defaults, either populate the section here or remove it to allow user overrides.• File: configuration/printers/base.cfg
– Line 48:[fastconfig]
– Line 50:[dynamic_governor]configuration/printers/v-core-3-hybrid/macros.cfg (1)
5-7: Delay increase looks goodBumping
initial_durationfrom 0.1 → 0.2 s brings this file in line with the other HYBRID/IDEX profiles and should avoid race conditions at startup.
No further concerns.configuration/moonraker.conf (1)
113-116: Pin update verified: commit exists upstreamThe pinned commit
93c8ad8a017c45f4502e73ad1dc174cf7d3c3b90on thedevchannel was confirmed to exist in the Klipper3d/klipper repository (HTTP 200). You can safely proceed with this pin, ensuring it includes the required Klipper features.configuration/printers/v-core-4-idex/v-core-4-idex.cfg (1)
7-9: Same duplication risk as aboveEnsure this additional
[gcode_macro RatOS]block is unique across all included files to prevent configuration load failures.configuration/printers/v-core-4-hybrid/v-core-4-hybrid.cfg (1)
7-9: LGTM! Beacon adaptive heat soak configuration added.The new
[gcode_macro RatOS]section withvariable_beacon_adaptive_heat_soak: Trueproperly enables the adaptive heat soak functionality for this printer configuration. This aligns with the coordinated enhancement across multiple printer variants to support Beacon-based thermal stability monitoring.configuration/printers/v-core-3/macros.cfg (1)
20-20: LGTM! Migration to fastconfig improves performance.The change from
printer.configfile.settings.beacontoprinter.fastconfig.settings.beaconis part of the coordinated migration to use the new fastconfig interface for improved configuration access performance. The beacon presence check logic remains unchanged.configuration/scripts/ratos-install.sh (1)
4-4: LGTM! Required dependencies added for new features.The addition of
libopenblas-baseandcpufrequtilsto the package list is necessary to support the new Klipper extensions:
libopenblas-baseprovides optimized numerical computations for the beacon adaptive heat soak modulecpufrequtilsenables the dynamic CPU governor functionalityconfiguration/macros/idex/util.cfg (2)
14-17: LGTM! Consistent fastconfig migration for stepper parameters.The migration from
printer.configfile.settingstoprinter.fastconfig.settingsfor stepper position parameters (position_max,position_endstop) improves performance while maintaining the same logic for dual carriage endstop calculations.
40-41: LGTM! Stepper Y parameters migrated to fastconfig.The change to use
printer.fastconfig.settingsforstepper_yposition parameters is consistent with the coordinated performance improvement across the macro system.configuration/macros/parking.cfg (1)
29-29: LGTM! Parking logic migrated to fastconfig.The change from
printer.configfile.settings.beacontoprinter.fastconfig.settings.beaconin the IDEX toolhead parking conditional maintains the existing logic while benefiting from the improved configuration access performance.configuration/macros/idex/idex_is.cfg (2)
46-46: LGTM! Configuration access modernized for better performance.The change from
printer.configfile.settingstoprinter.fastconfig.settingsaligns with the performance optimization initiative across the codebase.
306-306: LGTM! Consistent with performance optimization pattern.The migration to
printer.fastconfig.settingsfor accessing probe points maintains the same functionality while improving configuration access performance.configuration/printers/v-core-4-idex/macros.cfg (2)
15-15: LGTM! Timing adjustment for reliable IDEX initialization.Increasing the initial duration from 0.1 to 0.2 seconds provides more time for proper IDEX initialization before executing the hybrid verification.
42-42: LGTM! Consistent configuration access modernization.The migration to
printer.fastconfig.settingsfor beacon detection maintains functionality while improving performance.configuration/printers/v-core-4/macros.cfg (1)
16-16: LGTM! Standard configuration access optimization.The change to
printer.fastconfig.settingsfor beacon detection is consistent with the performance improvement initiative while maintaining identical functionality.configuration/printers/v-core-4-hybrid/macros.cfg (2)
6-6: LGTM! Improved initialization timing for hybrid configuration.Doubling the initial duration to 0.2 seconds provides adequate time for proper hybrid CoreXY configuration verification.
22-22: LGTM! Configuration access optimization implemented.The migration to
printer.fastconfig.settingsfor beacon detection maintains functionality while improving configuration access performance.configuration/printers/v-core-pro/macros.cfg (1)
16-16: LGTM! Final piece of configuration access modernization.The change to
printer.fastconfig.settingsfor beacon detection completes the consistent migration across all printer configurations, improving performance while maintaining functionality.configuration/printers/v-core-3-idex/macros.cfg (2)
14-14: LGTM - Coordinated initialization timing improvement.The increased initial duration from 0.1 to 0.2 seconds aligns with similar updates across other IDEX and hybrid printer configurations, suggesting this addresses initialization timing issues.
48-48: LGTM - Performance optimization through fastconfig.The transition from
printer.configfile.settings.beacontoprinter.fastconfig.settings.beaconis part of a system-wide optimization to use the new fastconfig module for faster configuration access while maintaining identical functionality.src/server/gcode-processor/Actions.ts (2)
99-99: LGTM - Added support for newer PrusaSlicer versions.The addition of versions 2.9.1 and 2.9.2 to the supported PrusaSlicer versions follows the established pattern and keeps the software current with recent releases.
101-101: LGTM - Updated error message to match supported versions.The error message correctly reflects the expanded version support, using a concise range format (2.9.0 - 2.9.2) that's consistent with the validation logic.
src/scripts/common.sh (1)
286-300: LGTM - Added sudoers whitelist for klippy extensions.The new klippy extension commands whitelist properly supports the dynamic CPU governor functionality by allowing
/usr/bin/cpufreq-setwithout password authentication. The implementation follows the established pattern used for other sudoers whitelists.configuration/macros/priming.cfg (1)
32-32: LGTM - System-wide migration to fastconfig for performance optimization.All changes consistently replace
printer.configfilewithprinter.fastconfigfor accessing configuration settings related to beacon, bltouch, probe, bed_mesh, and extruder configurations. This migration maintains identical functionality while improving configuration access performance through the new fastconfig module.Also applies to: 39-39, 42-44, 105-105, 108-108, 111-122, 207-207, 212-212, 262-262, 354-354, 379-379, 410-410
configuration/macros/idex/vaoc.cfg (2)
117-117: LGTM - Fastconfig migration for performance optimization.The changes from
printer.configfile.settingstoprinter.fastconfig.settingsfor dual_carriage.safe_distance and beacon configuration access align with the system-wide performance optimization while maintaining identical functionality.Also applies to: 304-304, 633-633
719-719: LGTM - Enhanced heat soak configurability.The addition of configurable
hotend_heat_soak_timewith conditional waiting logic improves flexibility by allowing users to customize thermal expansion wait times. The default value of 0 maintains backward compatibility while the conditional logic properly handles both enabled and disabled states.Also applies to: 736-740
configuration/scripts/ratos-common.sh (1)
194-198: LGTM! Proper extension registration for new modules.The addition of five new Klipper extensions follows the established pattern and uses consistent path resolution. This ensures proper validation and registration of the new modules introduced in this PR: Z-offset management, beacon true zero correction, adaptive heat soak, fast configuration access, and dynamic CPU governor.
configuration/klippy/fastconfig.py (1)
1-80: Excellent implementation of immutable configuration wrapper.This module provides a well-designed optimization to avoid expensive deep copies when accessing printer configuration in macros. Key strengths:
- Proper implementation of the Mapping interface
- Singleton-like copy semantics via
__copy__and__deepcopy__overrides- Appropriate initialization timing on
klippy:connectevent- Exclusion of transient
save_config_pendingkeys- Clear error handling for premature access
configuration/macros/idex/toolheads.cfg (4)
496-496: LGTM! Consistent migration to fastconfig.The changes properly migrate beacon presence and dual carriage configuration checks from
printer.configfile.settingstoprinter.fastconfig.settings, aligning with the performance optimization pattern used throughout this PR.Also applies to: 563-563, 779-780, 825-825
766-766: Appropriate timing adjustment for initialization sequencing.Increasing the initial duration from 0.1 to 0.2 seconds ensures proper startup sequencing, consistent with similar timing adjustments in other delayed_gcode macros across the codebase.
791-792: Enhanced debug logging improves traceability.The addition of
offset_beforevariable and enhanced debug messages with 6-decimal precision rounding provides better traceability of toolhead offset application. This complements the diagnostic improvements introduced throughout this PR.Also applies to: 811-811, 813-813, 816-816, 818-818, 821-821
830-833: Good separation of debug logging logic.The new
_SET_TOOLHEAD_OFFSET_LOG_DEBUGmacro properly separates debug logging functionality and provides clear before/after offset comparison with consistent precision formatting.configuration/macros/util.cfg (2)
48-50: Helpful clarification on macro execution timing.The updated comments clearly explain the initialization order requirements and timing constraints for delayed_gcode macros, which is valuable for maintainability.
58-58: Consistent migration to fastconfig pattern.The changes properly migrate configuration access from
printer.configfiletoprinter.fastconfigfor beacon presence, hybrid inverted flags, and skew correction profiles. This aligns with the performance optimization effort throughout the PR.Also applies to: 255-256, 383-383
configuration/homing.cfg (4)
81-81: Cleaner safe home position access.Using
printer.ratos.safe_home_positionwith tuple unpacking is more efficient and cleaner than separate calls for X and Y coordinates. This integrates well with the enhanced RatOS module capabilities.Also applies to: 124-124
155-156: Enhanced beacon calibration logic.The addition of
beacon_contact_start_print_true_zeroand associated fuzzing radius, along with the restructured calibration logic that distinguishes between start print and other calibration timing, provides better control over the beacon calibration workflow.Also applies to: 175-186
236-236: Consistent fastconfig migration.All configuration access has been properly migrated from
printer.configfile.configtoprinter.fastconfig.config, maintaining consistency with the performance optimization pattern throughout the PR.Also applies to: 240-240, 247-247, 251-251, 302-302, 306-306, 313-313, 317-317, 329-330
334-336: Appropriate refactoring of move logic.The rename to
__MOVE_TO_SAFE_Z_HOME_ECHO_DEBUGand simplification to debug-only functionality suggests the actual move logic has been properly moved to the Python RatOS module, improving the separation of concerns.configuration/macros/overrides.cfg (6)
95-108: Beacon nozzle temperature offset integration looks good.The conditional application of beacon nozzle temperature offsets is properly implemented with appropriate guards for beacon existence and print state.
149-153: LGTM! Beacon temperature offset properly applied after temperature wait.The implementation correctly updates the nozzle thermal expansion offset after the temperature wait completes, maintaining consistency with the M104 implementation.
166-191: Beacon integration in SET_HEATER_TEMPERATURE is well implemented.The toolhead tracking and conditional beacon offset application maintains consistency with other temperature-related macros.
223-247: Excellent error handling improvements in TEMPERATURE_WAIT.The enhanced logging with clear sensor/toolhead identification and the explicit handling of missing parameters improves debugging and maintains consistent behavior by allowing the base macro to raise the expected error.
264-264: Fastconfig migration implemented correctly.The change from
configfile.settingstofastconfig.settingsaligns with the broader performance optimization effort.
319-354: Well-designed BED_MESH_CALIBRATE wrapper implementation.The macro properly handles pre-calibration requirements (Z tilt/QGL), beacon-specific validations, and stowable probe management. The conditional logic is clear and the implementation follows established patterns.
configuration/klippy/ratos_z_offset.py (1)
93-105: Transform implementation is correct.The get_position and move methods properly handle Z offset application and removal, following the expected gcode_move transform pattern.
configuration/klippy/dynamic_governor.py (3)
198-254: Well-designed non-blocking subprocess execution.The implementation correctly handles subprocess execution without blocking Klipper's reactor, with proper timeout and cleanup handling. The polling approach is appropriate for the single-threaded event loop.
149-170: Robust stepper state tracking implementation.The enabled stepper counting and governor switching logic is well-designed, with appropriate hysteresis via the idle delay timer to prevent governor thrashing during normal operations.
161-163: Good defensive programming against count inconsistencies.The guard against negative enabled counts is appropriate defensive programming, protecting against potential callback ordering issues or duplicate state changes.
configuration/macros/mesh.cfg (5)
15-46: Comprehensive bed mesh validation logic.The sanity checks properly validate zero reference position compatibility with beacon features and provide clear, actionable error messages. The warnings for non-recommended beacon settings are helpful.
48-56: Clean preflight check implementation.The macro properly resets internal state and conditionally performs beacon-specific preflight checks.
74-79: Well-implemented zero reference position handling.The logic properly handles undefined zero reference positions with appropriate fallbacks to safe home or actual true zero positions. The explicit setting of zero reference after mesh operations ensures consistency.
Also applies to: 103-103, 121-128
178-191: Excellent zero reference position expansion logic.The implementation ensures the zero reference position is always within the mesh region by expanding boundaries as needed. This is essential for proper bed mesh referencing.
275-279: Smart probe order adjustment for beacon mesh runs.The logic that adjusts probe order based on beacon mesh run count parity helps distribute wear patterns when using beacon contact probing.
configuration/klippy/beacon_true_zero_correction.py (1)
176-176: Cross-platform path handling & f-string cleanupPlease address two issues in
configuration/klippy/beacon_true_zero_correction.py:
- Avoid hard-coding
/home/pi/printer_data/.... In Python,~won’t auto-expand, and different users or systems won’t have that path. Instead, read the base path from theRATOS_PRINTER_DATA_DIRenvironment variable (set inconfiguration/scripts/environment.sh) or useos.path.expanduser("~"), thenos.path.join(...)to build the full filepath.- Remove the unnecessary
fprefix on strings without placeholders (e.g.f"Unknown action."→"Unknown action.").Key locations to update:
- ~Line 176: change
to something like:filename = f'/home/pi/printer_data/config/mpp_capture_{timestamp}.csv'import os base = os.environ.get("RATOS_PRINTER_DATA_DIR", os.path.expanduser("~") + "/printer_data") filename = os.path.join(base, "config", f"mpp_capture_{timestamp}.csv")- The
raise self.gcode.error(f"Unknown action.")call: drop thefso it readsraise self.gcode.error("Unknown action.")Please verify these changes and ensure the code works across all supported environments.
configuration/klippy/ratos.py (3)
238-243: LGTM! Clean error handling implementationThe
_RAISE_ERRORcommand provides a cleaner way to raise errors from macros without the unhelpful macro name prefixing.
245-278: Well-implemented try/except/finally patternThe
_TRYcommand provides robust error handling for G-code execution with proper exception propagation and cleanup.
651-695: Excellent implementation of fuzzy positioningThe
_MOVE_TO_SAFE_Z_HOMEcommand with fuzzy positioning is well-implemented with proper boundary checking and uniform distribution within the circle.configuration/macros.cfg (6)
283-295: Good addition of LED error signalingAdding
_LED_START_PRINTING_ERRORbefore raising errors provides better visual feedback to users when mandatory parameters are missing.
359-364: Proper preflight checks for beacon functionalityThe preflight checks ensure beacon model exists and bed mesh variables are reset before printing starts.
444-447: Performance improvement with fastconfig migrationGood migration from
printer.configfile.settingstoprinter.fastconfig.settingsfor improved performance. The new fastconfig module provides immutable snapshots that are more efficient to access.Also applies to: 487-487
625-639: Well-integrated beacon adaptive heat soakThe adaptive heat soak integration properly positions the toolhead close to the bed for accurate measurements and respects all configuration parameters.
734-741: Good state management for toolhead standbyResetting standby flags ensures consistent state before conditionally setting standby mode based on configuration.
1274-1281: Correct beacon offset handling in END_PRINTThe change from
SET_GCODE_OFFSET Z=0 MOVE=0to_BEACON_SUBTRACT_CONTACT_EXPANSION_OFFSETproperly handles beacon expansion compensation when enabled.configuration/klippy/beacon_mesh.py (4)
15-63: Well-structured constants with clear documentationThe constants are well-organized with consistent naming and helpful comments explaining each parameter's purpose. The categorization into mesh kinds, probe methods, and parameters provides a clean API for mesh management.
84-109: Good initialization pattern with deferred component loadingThe design correctly defers component loading to the klippy:connect event handler, preventing initialization failures if components aren't available early. The separation of concerns between initialization and component loading is well-implemented.
300-368: Excellent error handling and user guidance in auto-selection logicThe method provides comprehensive error handling with clear, actionable messages for users. The logic correctly handles edge cases like missing profiles, ambiguous selections, and temperature mismatches. The inclusion of documentation links is particularly helpful.
467-546: Comprehensive parameter validation with excellent user feedbackThe validation logic is thorough and covers all edge cases with clear, specific error messages. The use of allowed lists for kinds and probe methods provides good flexibility, and the temperature comparison logic with configurable error/warning behavior is well-designed.
configuration/z-probe/beacon.cfg (3)
54-134: Excellent configuration documentation and thoughtful defaultsThe configuration variables are exceptionally well-documented with clear explanations of each parameter's purpose and impact. The adaptive heat soak quality levels (1-5) provide intuitive control over the trade-off between speed and quality. The fuzzy position feature for preventing bed wear is a thoughtful addition.
654-743: Sophisticated IDEX-aware thermal expansion compensationThe implementation correctly handles all IDEX modes with appropriate offset application logic. The detailed debug logging will be invaluable for troubleshooting thermal compensation issues. The separation of offset calculation and application provides good maintainability.
1175-1235: Well-designed offset management system with proper component separationThe offset management system elegantly separates automatic offsets (thermal expansion, IDEX toolhead) from user manual adjustments. The calculation logic correctly accounts for all offset sources, and the debug logging provides excellent visibility into the offset calculations. This design allows preserving user Z adjustments across prints while still applying automatic compensations.
| import time, logging, os, multiprocessing, traceback, pygam | ||
| import numpy as np |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify pygam dependency is properly installed
The module imports pygam which is required for the GAM (Generalized Additive Model) functionality. Ensure this dependency is properly documented and installed in the system.
🏁 Script executed:
#!/bin/bash
# Check if pygam is listed in requirements or installation scripts
rg -i "pygam" --type-add 'req:*.{txt,requirements}' --type-add 'script:*.sh' -t req -t scriptLength of output: 93
🏁 Script executed:
#!/bin/bash
# Search for any mention of `pygam` in the repository
rg -in "pygam"Length of output: 667
Add missing pygam dependency to project manifests
The code in configuration/klippy/beacon_adaptive_heat_soak.py imports and uses pygam (lines 7, 117–122), but no references to pygam were found in any requirements or installation scripts. Please ensure the dependency is declared and installed:
- Add
pygamto your project’s dependency file (e.g.,requirements.txt,setup.py,Pipfile, orpyproject.toml). - Update any installation documentation or CI setup to include
pygam.
🤖 Prompt for AI Agents
In configuration/klippy/beacon_adaptive_heat_soak.py around lines 7 to 8, the
module imports pygam but this dependency is not declared in any project manifest
files. To fix this, add pygam to the appropriate dependency file such as
requirements.txt, setup.py, Pipfile, or pyproject.toml depending on your project
setup. Also update any installation instructions or CI configuration to ensure
pygam is installed during setup.
| path = os.path.join( | ||
| os.path.dirname(os.path.realpath(__file__)), | ||
| 'beacon_adaptive_heat_soak_model_training.csv') | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use more robust path resolution
The hardcoded path construction could fail if the module is moved or packaged differently. Consider using a more robust approach.
- path = os.path.join(
- os.path.dirname(os.path.realpath(__file__)),
- 'beacon_adaptive_heat_soak_model_training.csv')
+ # Get the directory containing this module
+ module_dir = os.path.dirname(os.path.realpath(__file__))
+ path = os.path.join(module_dir, 'beacon_adaptive_heat_soak_model_training.csv')
+
+ # Also check in the config directory as a fallback
+ if not os.path.exists(path):
+ config_path = os.path.join(self.printer.get_config_dir(), 'beacon_adaptive_heat_soak_model_training.csv')
+ if os.path.exists(config_path):
+ path = config_pathCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In configuration/klippy/beacon_adaptive_heat_soak.py around lines 83 to 86, the
current path construction using os.path.join with
os.path.dirname(os.path.realpath(__file__)) is fragile and may break if the
module is relocated or packaged differently. Replace this with a more robust
method such as using pathlib.Path with the __file__ attribute to resolve the
path relative to the current file, ensuring compatibility across different
environments and packaging setups.
| logging.info(f"{self.name}: Prepared for sampling, collected {good_samples} good samples and {bad_samples} bad samples (total {good_samples+bad_samples} samples).") | ||
|
|
||
| if good_samples < 1000: | ||
| raise self.printer.command_error(f"Failed to prepare beacon for sampling, timed out waiting for good samples. Beacon must be calibrated and positioned correctly before running this command.") |
There was a problem hiding this comment.
Remove unnecessary f-string prefix
The f-string prefix is not needed here as there are no placeholders in the string.
- raise self.printer.command_error(f"Failed to prepare beacon for sampling, timed out waiting for good samples. Beacon must be calibrated and positioned correctly before running this command.")
+ raise self.printer.command_error("Failed to prepare beacon for sampling, timed out waiting for good samples. Beacon must be calibrated and positioned correctly before running this command.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| raise self.printer.command_error(f"Failed to prepare beacon for sampling, timed out waiting for good samples. Beacon must be calibrated and positioned correctly before running this command.") | |
| raise self.printer.command_error("Failed to prepare beacon for sampling, timed out waiting for good samples. Beacon must be calibrated and positioned correctly before running this command.") |
🧰 Tools
🪛 Ruff (0.12.2)
381-381: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In configuration/klippy/beacon_adaptive_heat_soak.py at line 381, remove the
unnecessary f-string prefix from the string in the raise statement since there
are no placeholders to format. Change the string from an f-string to a regular
string literal.
| z_rate_result = z_rate_session.get_next_z_rate() | ||
| except Exception as e: | ||
| if self.printer.is_shutdown(): | ||
| raise | ||
| else: | ||
| raise self.printer.command_error(f"Error calculating Z-rate, wait ended prematurely: {e}") | ||
|
|
There was a problem hiding this comment.
Use proper exception chaining
When re-raising exceptions within an except clause, use raise ... from to maintain the exception chain.
except Exception as e:
if self.printer.is_shutdown():
raise
else:
- raise self.printer.command_error(f"Error calculating Z-rate, wait ended prematurely: {e}")
+ raise self.printer.command_error(f"Error calculating Z-rate, wait ended prematurely: {e}") from e📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| z_rate_result = z_rate_session.get_next_z_rate() | |
| except Exception as e: | |
| if self.printer.is_shutdown(): | |
| raise | |
| else: | |
| raise self.printer.command_error(f"Error calculating Z-rate, wait ended prematurely: {e}") | |
| z_rate_result = z_rate_session.get_next_z_rate() | |
| except Exception as e: | |
| if self.printer.is_shutdown(): | |
| raise | |
| else: | |
| raise self.printer.command_error( | |
| f"Error calculating Z-rate, wait ended prematurely: {e}" | |
| ) from e |
🧰 Tools
🪛 Ruff (0.12.2)
520-520: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In configuration/klippy/beacon_adaptive_heat_soak.py around lines 515 to 521,
the exception is re-raised without proper chaining, which loses the original
traceback. Modify the raise statement to use 'raise ... from e' to preserve the
original exception context when raising self.printer.command_error.
| fullpath = f'/home/pi/printer_data/config/{filename}' | ||
|
|
There was a problem hiding this comment.
Security concern: Hardcoded file paths
The diagnostic commands write to hardcoded paths in the user's home directory, which could be a security risk if the user is not 'pi'.
Consider using the printer's config directory or a configurable path:
- fullpath = f'/home/pi/printer_data/config/{filename}'
+ config_dir = self.printer.get_config_dir()
+ fullpath = os.path.join(config_dir, filename)Similarly for line 678:
- filename = f'/home/pi/printer_data/config/beacon_adaptive_heat_soak_beacon_samples_{timestamp}.txt'
+ config_dir = self.printer.get_config_dir()
+ filename = os.path.join(config_dir, f'beacon_adaptive_heat_soak_beacon_samples_{timestamp}.txt')Also applies to: 678-679
🤖 Prompt for AI Agents
In configuration/klippy/beacon_adaptive_heat_soak.py at lines 624-625 and
678-679, the code uses hardcoded file paths pointing to
'/home/pi/printer_data/config/', which is insecure if the user is not 'pi'.
Replace these hardcoded paths by dynamically obtaining the printer's config
directory or use a configurable path variable to construct the full file paths,
ensuring the code works correctly regardless of the user's home directory.
| self.tzc.ratos.console_echo( | ||
| RATOS_TITLE, | ||
| 'error', | ||
| f'The measured true zero correction {self._finalize_result:.6f} is below the safety limit of -0.2mm._N_This is not expected behaviour.') | ||
| raise self.gcmd.error(f'Measured correction is below safety limit') | ||
| logging.info(f'{self.tzc.name}: applying correction {self._finalize_result:.6f}') | ||
| self.gcmd.respond_info(f'Applying true zero correction of {self._finalize_result*1000.:.1f} µm') | ||
| self.tzc.ratos_z_offset.set_offset('true_zero_correction', self._finalize_result) |
There was a problem hiding this comment.
Good safety check, but fix f-string.
The -0.2mm safety limit is a good protection against bed damage. Remove the unnecessary f-string prefix.
- raise self.gcmd.error(f'Measured correction is below safety limit')
+ raise self.gcmd.error('Measured correction is below safety limit')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.tzc.ratos.console_echo( | |
| RATOS_TITLE, | |
| 'error', | |
| f'The measured true zero correction {self._finalize_result:.6f} is below the safety limit of -0.2mm._N_This is not expected behaviour.') | |
| raise self.gcmd.error(f'Measured correction is below safety limit') | |
| logging.info(f'{self.tzc.name}: applying correction {self._finalize_result:.6f}') | |
| self.gcmd.respond_info(f'Applying true zero correction of {self._finalize_result*1000.:.1f} µm') | |
| self.tzc.ratos_z_offset.set_offset('true_zero_correction', self._finalize_result) | |
| self.tzc.ratos.console_echo( | |
| RATOS_TITLE, | |
| 'error', | |
| f'The measured true zero correction {self._finalize_result:.6f} is below the safety limit of -0.2mm._N_This is not expected behaviour.') | |
| raise self.gcmd.error('Measured correction is below safety limit') | |
| logging.info(f'{self.tzc.name}: applying correction {self._finalize_result:.6f}') | |
| self.gcmd.respond_info(f'Applying true zero correction of {self._finalize_result*1000.:.1f} µm') | |
| self.tzc.ratos_z_offset.set_offset('true_zero_correction', self._finalize_result) |
🧰 Tools
🪛 Ruff (0.12.2)
393-393: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In configuration/klippy/beacon_true_zero_correction.py around lines 389 to 396,
the f-string used in the console_echo call includes no variable interpolation
and is unnecessary. Remove the f-string prefix (the leading 'f') from the string
literal to fix this issue, keeping the string content unchanged.
| raise self.printer.config_error( | ||
| f"{self.name}: cpufreq-set command not found. " | ||
| "Please install the cpufrequtils package and ensure it is in the PATH." | ||
| ) | ||
| except subprocess.CalledProcessError as e: | ||
| raise self.printer.config_error( | ||
| f"{self.name}: cpufreq-set command failed: {e}. " | ||
| "Please ensure cpufrequtils is installed correctly." | ||
| ) | ||
| except Exception as e: | ||
| raise self.printer.config_error( | ||
| f"{self.name}: Unexpected error checking cpufreq-set: {e}. " | ||
| "Please ensure cpufrequtils is installed correctly." | ||
| ) |
There was a problem hiding this comment.
Add exception chaining for better error context.
The exception handling should use from e to preserve the original exception context, which aids in debugging.
except FileNotFoundError:
raise self.printer.config_error(
f"{self.name}: cpufreq-set command not found. "
"Please install the cpufrequtils package and ensure it is in the PATH."
- )
+ ) from None
except subprocess.CalledProcessError as e:
raise self.printer.config_error(
f"{self.name}: cpufreq-set command failed: {e}. "
"Please ensure cpufrequtils is installed correctly."
- )
+ ) from e
except Exception as e:
raise self.printer.config_error(
f"{self.name}: Unexpected error checking cpufreq-set: {e}. "
"Please ensure cpufrequtils is installed correctly."
- )
+ ) from eApply the same pattern to the cpufreq-info exception handlers in lines 118-131.
Also applies to: 118-131
🧰 Tools
🪛 Ruff (0.12.2)
86-89: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
91-94: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
96-99: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
In configuration/klippy/dynamic_governor.py around lines 86 to 99, update the
exception handling to use exception chaining by adding 'from e' to each raise
statement. This preserves the original exception context for better debugging.
Similarly, apply the same 'from e' pattern to the exception handlers for
cpufreq-info in lines 118 to 131 to maintain consistency and improve error
traceability.
| def cmd_SET_RATOS_Z_OFFSET(self, gcmd): | ||
| name = gcmd.get('NAME').lower().strip() | ||
| if name not in OFFSET_NAMES: | ||
| raise gcmd.error(f"Offset name '{name}' is not recognized.") | ||
| offset = gcmd.get_float('OFFSET') | ||
| if offset == 0.: | ||
| self.offsets.pop(name, None) | ||
| else: | ||
| self.offsets[name] = offset | ||
| self._offset_changed() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicated offset validation logic.
The validation logic for offset names and zero-value handling is duplicated between cmd_SET_RATOS_Z_OFFSET and set_offset. Consider extracting this to a shared method.
+ def _validate_and_set_offset(self, name, offset):
+ """Validate and set an offset value."""
+ if name not in OFFSET_NAMES:
+ raise self.gcode.error(f"Offset name '{name}' is not recognized.")
+ if offset == 0.:
+ self.offsets.pop(name, None)
+ else:
+ self.offsets[name] = float(offset)
+
desc_SET_RATOS_Z_OFFSET = "Set a RatOS Z offset"
def cmd_SET_RATOS_Z_OFFSET(self, gcmd):
name = gcmd.get('NAME').lower().strip()
- if name not in OFFSET_NAMES:
- raise gcmd.error(f"Offset name '{name}' is not recognized.")
offset = gcmd.get_float('OFFSET')
- if offset == 0.:
- self.offsets.pop(name, None)
- else:
- self.offsets[name] = offset
+ self._validate_and_set_offset(name, offset)
self._offset_changed()
# For use by other extensions
def set_offset(self, name:str, offset:float):
if name:
name = name.strip().lower()
- if name not in OFFSET_NAMES:
- raise self.gcode.error(f"Offset name '{name}' is not recognized.")
- if offset == 0.:
- self.offsets.pop(name, None)
- else:
- self.offsets[name] = float(offset)
+ self._validate_and_set_offset(name, offset)
self._offset_changed()Also applies to: 79-88
🤖 Prompt for AI Agents
In configuration/klippy/ratos_z_offset.py around lines 47-56 and 79-88, the
offset name validation and zero-value handling logic is duplicated in
cmd_SET_RATOS_Z_OFFSET and set_offset methods. Refactor by extracting this
common validation and update logic into a single private helper method that both
functions can call to reduce code duplication and improve maintainability.
| [gcode_macro RatOS] | ||
| variable_beacon_adaptive_heat_soak: True | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential duplicate [gcode_macro RatOS] section
Adding a second [gcode_macro RatOS] (one may already live in base.cfg or other includes) will cause Klipper to abort with “Section … already defined”.
Please verify only one instance exists in the final merged config:
If duplication is unavoidable, rename this macro (e.g., [gcode_macro RatOS_PRINTER]) and adjust macro calls.
🏁 Script executed:
rg -n '^\[gcode_macro RatOS\]' configuration/printersLength of output: 1696
Prevent duplicate [gcode_macro RatOS] definitions
Klipper aborts when the same [gcode_macro] section is defined more than once. Our grep shows multiple occurrences of [gcode_macro RatOS] across printer configs (e.g., v-core-4, v-core-4-idex, v-core-pro, v-minion, voron-v24, etc.), so this needs to be addressed.
Choose one of the following fixes:
- Centralize the macro into a single shared include (e.g.,
base.cfg), then remove all other[gcode_macro RatOS]blocks. - Rename the macro in this file to a unique name and update all calls. For example, in
configuration/printers/v-core-4/v-core-4.cfg:
--- a/configuration/printers/v-core-4/v-core-4.cfg
+++ b/configuration/printers/v-core-4/v-core-4.cfg
-[gcode_macro RatOS]
+[gcode_macro RatOS_VCORE_4]
variable_beacon_adaptive_heat_soak: TrueAffected file:
- configuration/printers/v-core-4/v-core-4.cfg:7
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [gcode_macro RatOS] | |
| variable_beacon_adaptive_heat_soak: True | |
| [gcode_macro RatOS_VCORE_4] | |
| variable_beacon_adaptive_heat_soak: True |
🤖 Prompt for AI Agents
In configuration/printers/v-core-4/v-core-4.cfg around lines 7 to 9, there is a
duplicate [gcode_macro RatOS] definition causing Klipper to abort. To fix this,
either move the [gcode_macro RatOS] block to a single shared include file like
base.cfg and remove it from this file, or rename the macro here to a unique name
and update all references to it accordingly to avoid conflicts.
There was a problem hiding this comment.
@coderabbitai this isn't true. Klipper uses config section merging, with latter section parameters (chronologically) taking precedence over earlier ones. Please add this to your memory for future reviews.
There was a problem hiding this comment.
@miklschmidt Thank you for the correction! I understand now that Klipper uses config section merging rather than throwing errors on duplicate sections. I appreciate you clarifying this behavior and will remember it for future reviews.
✏️ Learnings added
Learnt from: miklschmidt
PR: Rat-OS/RatOS-configurator#101
File: configuration/printers/v-core-4/v-core-4.cfg:7-9
Timestamp: 2025-07-28T19:09:04.968Z
Learning: In Klipper configuration files, duplicate sections (like `[gcode_macro RatOS]`) do not cause errors. Instead, Klipper uses config section merging where later section parameters (chronologically) take precedence over earlier ones. This means multiple definitions of the same section are allowed and the last one processed will override parameters from earlier definitions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: LucidityCrash
PR: Rat-OS/RatOS-configurator#89
File: src/templates/v-core-pro.ts:18-20
Timestamp: 2025-06-16T16:44:30.280Z
Learning: In the RatOS configurator project, v-core-pro configuration files are stored in `configuration/printers/v-core-pro/` in the source repository, but are accessed via `RatOS/printers/v-core-pro/` paths in the deployed/installed environment. The include statements in templates should use the runtime paths, not the source repository paths.
src/scripts/common.sh
Outdated
| $sudo chown root:root /tmp/031-ratos-configurator-scripts | ||
| $sudo chmod 440 /tmp/031-ratos-configurator-scripts | ||
| $sudo cp --preserve=mode /tmp/031-ratos-configurator-scripts /etc/sudoers.d/031-ratos-configurator-scripts | ||
|
|
||
| echo "RatOS configurator scripts has successfully been whitelisted!" |
There was a problem hiding this comment.
Remove duplicated configurator scripts whitelist setup.
These lines duplicate the configurator scripts whitelist setup that was already performed earlier in the function (lines 257-261). This redundant code should be removed to avoid confusion and unnecessary operations.
- $sudo chown root:root /tmp/031-ratos-configurator-scripts
- $sudo chmod 440 /tmp/031-ratos-configurator-scripts
- $sudo cp --preserve=mode /tmp/031-ratos-configurator-scripts /etc/sudoers.d/031-ratos-configurator-scripts
-
- echo "RatOS configurator scripts has successfully been whitelisted!"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $sudo chown root:root /tmp/031-ratos-configurator-scripts | |
| $sudo chmod 440 /tmp/031-ratos-configurator-scripts | |
| $sudo cp --preserve=mode /tmp/031-ratos-configurator-scripts /etc/sudoers.d/031-ratos-configurator-scripts | |
| echo "RatOS configurator scripts has successfully been whitelisted!" |
🤖 Prompt for AI Agents
In src/scripts/common.sh around lines 280 to 284, the configurator scripts
whitelist setup is duplicated from lines 257 to 261. Remove these redundant
lines to avoid repeating the same chown, chmod, and cp commands, ensuring the
whitelist setup is done only once in the function.
…rrors - pass the reactor instance on to directly-constructed ZMesh objects - use a non-zero waketime for reactor.pause()
- Sometimes we get a very noisy signal on a machine that otherwise gives a normal signal. This is not well understood. We see some correlation with soaking from warm. Informed by extensive data analysis, this fix adds a second level moving average which smooths the noise and is also tested to determine if the soak is complete. This approach may be revised in the future if/when the noise behaviour is better understood.
- this slightly extends the implied practical noise floor of the system, and is in response to continued usage and data gathering
…ulative workaround for occasional failures that have been observed
… ratos.py for general reuse
- This replaces the previous implementation that was susceptible to non-uniform thermal effects observed in extended testing.
…mpensation mesh in non-automated workflow
- This can produce a visible improvement in surface quality, and appears to have little if any impact on CPU usage. The change was not applied more widely as other machines may have lower spec host boards, and the change has not been tested on those machines.
During further testing it became clear that changing the governor while klipper is running can occasionally lead to timer too close or other driver communication issues, and for the time being no workaround was found. So removing the feature for now.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores