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

StartSliceJob.py regex - tweak to extruder_nr_expr capture group #20203

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DragRedSim
Copy link

Description

Previously, any expression in a G-code block that was to be evaluated at slicing time could not contain a comma, as the regex would treat everything before the first comma found as the "expression", and anything after it as the "extruder_nr_expr". This causes a problem when trying to evaluate an expression that takes multiple values; for example, one such as {min(machine_max_feedrate_x, machine_max_feedrate_y)*60, -1}, which calculates a feedrate for moving the toolhead as part of the preparation steps. This change alters the selection of the "extruder_nr_expr" value to only find a string composed of numbers and the minus sign, in order to ensure this field only contains the extruder number in use, and thus allows the capture of the "expression" value to extend past the first comma, to take in the entire expression.

This fixes any expressions being interpreted by the parser which involve the use of a function that takes more than one argument.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Printer definition file(s)
  • Translations

How Has This Been Tested?

  • Test A - attempted slicing using a stock install of Cura 5.9.0, with the following line in the machine start G-code: G0 X-2.1 Y20 F{min(machine_max_feedrate_x, machine_max_feedrate_y)*60, -1} ;Move to start position. This line was output in the sliced gcode as G0 X-2.1 Y20 FNone ;Move to start position

  • Test B - made change in the installed file StartSliceJob.py as documented in this pull request and restarted Cura, re-sliced the same file and checked the line in the start G-code, which now outputs G0 X-2.1 Y20 F12000 ;Move to start position

Test Configuration:

  • Operating System: Windows 11

Checklist:

Previously, any expression in a G-code block that was to be evaluated at slicing time could not contain a comma, as the regex would treat everything before the first comma found as the "expression", and anything after it as the "extruder_nr_expr". This causes a problem when trying to evaluate an expression that takes multiple values; for example, one such as `{min(machine_max_feedrate_x, machine_max_feedrate_y)*60, -1}`, which calculates a feedrate for moving the toolhead as part of the preparation steps.
This change alters the selection of the "extruder_nr_expr" value to only find a string composed of numbers and the minus sign, in order to ensure this field only contains the extruder number in use, and thus allows the capture of the "expression" value to extend past the first comma, to take in the entire expression.
@github-actions github-actions bot added the PR: Community Contribution 👑 Community Contribution PR's label Jan 28, 2025
Copy link
Contributor

github-actions bot commented Jan 28, 2025

Test Results

1 tests   - 23 379   0 ✅  - 23 378   2s ⏱️ -43s
1 suites ±     0   0 💤  -      2 
1 files   ±     0   0 ❌ ±     0   1 🔥 +1 

For more details on these errors, see this check.

Results for commit 6326431. ± Comparison against base commit ccbd537.

This pull request removes 23380 and adds 1 tests. Note that renamed tests count towards both.
tests.API.TestAccount ‑ test_errorLoginState
tests.API.TestAccount ‑ test_initialize
tests.API.TestAccount ‑ test_login
tests.API.TestAccount ‑ test_logout
tests.API.TestAccount ‑ test_sync_success
tests.API.TestAccount ‑ test_sync_update_action
tests.Machines.Models.TestDiscoveredCloudPrintersModel ‑ test_discoveredCloudPrinters
tests.Machines.Models.TestDiscoveredPrintersModel ‑ test_discoveredPrinters
tests.Machines.Models.TestDiscoveredPrintersModel ‑ test_getAndSet[data0]
tests.Machines.Models.TestDiscoveredPrintersModel ‑ test_getAndSet[data1]
…
tests.Machines.TestStartEndGCode

♻️ This comment has been updated with latest results.

Since my first idea failed to pass testing (I didn't consider that people may want to also calculate the extruder number), here's a different approach by recursively capturing the contents of brackets to ensure matched pairs (necessary for a valid expression to be evaluated).
@DragRedSim
Copy link
Author

OK, so turns out when I was developing the regex I didn't realise the re package used by Cura in this segment of code does not have support for the recursive function call I was using to ensure that all brackets were positively matched. The alternative regex library, however, does have support for this feature, however it is a third-party dependency, rather than the second-party one currently in use (as re is a core module of Python, whereas regex requires installation). I don't think there's much more I can do with this at the current point, not without manually parsing the string (basically counting brackets and looking for commas) to determine where a complete expression is able to be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Community Contribution 👑 Community Contribution PR's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant