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

Create PurgeLinesAndUnload.py #20013

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

GregValiant
Copy link
Collaborator

@GregValiant GregValiant commented Dec 9, 2024

Description

This script has 4 options.
Add Purge Lines:
Will draw lines left, right, top, or bottom of the build plate and either full length of half length. If a print takes up the entire width then the purge lines could be moved to the bottom.
Circle Around To Layer Start:
Creates an orthogonal tool path that moves the nozzle around the periphery of the build plate before moving in to the Layer Start location. This keeps strings from dragging across the print location on the build plate.
Adjust Starting E Location:
Cura adds a retraction after the StartUp Gcode. If there is also a retraction within the StartUp Gcode (or when using Add Purge Lines) it causes a double retraction. This option will change that "G1 E-" line to a "G92 E" so the filament location within the nozzle can be adjusted to provide for an exact start to the skirt/brim/raft.
Unload FIlament at Print End:
Adds an unload sequence of G1 E commands to back the filament out of the hot end and out of the extruder. Works well with long-tube bowden printers.

The script supports:
Absolute and Relative extrusion
Rectangular and Elliptic beds
Origin-At-Center whether True or False.
Disallowed Areas

Settings

image

Type of change

  • [ X] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I've been using this script for a year.

Test Configuration:
Dell Laptop 16mb of RAM

  • Operating System:
    Windows 10 Pro
    Cura versions 4.13.1 and up.

Checklist:

  • [ X] My code follows the style guidelines of this project as described in UltiMaker Meta and Cura QML best practices
  • [ X] I have read the Contribution guide
  • [ X] I have commented my code, particularly in hard-to-understand areas
  • [ X] I have uploaded any files required to test this change
  • [ X] I apologize ahead of time for my coding style.

This script has 4 options.
Add Purge Lines will draw lines left, right, top, or bottom of the build plate and either fill length of half length.  If a print takes up the entire width then the purge lines could be moved to the bottom.
@github-actions github-actions bot added the PR: Community Contribution 👑 Community Contribution PR's label Dec 9, 2024
Copy link
Contributor

github-actions bot commented Dec 9, 2024

Test Results

23 370 tests  ±0   23 368 ✅ ±0   53s ⏱️ -1s
     1 suites ±0        2 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 79ec595. ± Comparison against base commit 382b98e.

♻️ This comment has been updated with latest results.

@GregValiant
Copy link
Collaborator Author

@HellAholic the "Move around to the print start" function is something that came up in a post a while ago. I did account for cases where Cura adds the "Move to prime tower" line at the end of the startup. The script will add an orthogonal move to an edge before the actual move to the prime tower location.
I have high confidence in the code for "Rectangular" beds. The G2/G3 coding in the "Elliptic" section seems OK but needs another set of eyeballs.
I'll leave this as a draft for now.

@GregValiant GregValiant marked this pull request as draft December 9, 2024 02:11
@HellAholic
Copy link
Contributor

I'll take a look to see if I can update the code style. Will also try to play around with the current iteration to first get a grasp on how things work and what the output looks like. Might be a slow burner but it's on my todo list 👍

Changed 'Execute' procedure per suggestion.
Add 'G10' firmware retraction support to 'Adjust Starting E'.
@HellAholic
Copy link
Contributor

HellAholic commented Dec 14, 2024

I went with the purge everything approach and it started working, something else I had in the background was interfering with the script.
On the functionality side, would it be an idea to account for the disallowed area of the build plate?
[extreme example chosen for demonstration purposes] on the method series, the nozzle/extruder physically cannot reach there. Don't think there are any other printers with that type of restriction. But just as a discussion point, might be something to consider.
The worry I have with that (disallowed area) is then you might open up the whole shrinkage factor compensation thing, but it's just a worry, since you're applying the position in post processing, it does not get modified by the engine, so just as something to maybe check later if the disallowed area logic is introduced.

image

Update:
Had a bit of a though about it, we could add an extra field (offset from the edge), this would allow for adjustments in case you would want to adjust for the disallowed area or maybe you have a build plate with damaged edges and you want to do the purge a bit more towards the inside.
Also I'm a bit annoyed by the preview not showing the post processing results and having to load things back in to check. I'll see if I can do some adjustments to re-load the preview with the changes.

@GregValiant
Copy link
Collaborator Author

Good morning.
I missed the "machine_disallowed_areas" thing. After looking at the MethodX disallowed areas I think simply excluding 'Purge Lines' and 'Move to Start' is the best way to go. The polygons in a definition file can be written differently by different authors. They are easy to draw, but tough to parse.
The same thing might be true for any multi-extruder machine that does not "Share Heater" and "Share Nozzle since both the Purge Lines and the Unload will be for the active nozzle.
I'll spend some time thinking on those.

Regarding not showing the post-processed gcode in the initial preview, I recall seeing complaints about that here in the bug reports. The Initial Preview is done before any post-processing so the file would need to be saved, and then the gcode read back in. That'll take a while. Maybe a "Load Gcode file for Preview" button (that popped up after saving, or became yet another function of the "Slice" button) would be better so the user would have an option. (If Arc-Welder is enabled then it won't show correctly anyway.)

On a different subject...
It would be nice if there was a ToolTip attached to each name in the "Post Processing Scripts" list. Something like:
image
You would still need to load the PP to see the description, but there would be an indication of what it actually does.

@HellAholic
Copy link
Contributor

HellAholic commented Dec 14, 2024

Aloha,
Let me know what you come up with, you can always add "single extruder only" to the script or something.
I checked the offset, it works somewhat, but also needs a secondary field for start and end of the purge line. There are some general patterns in how the gcode is made, so if I can make a template out of it, then adjusting the values would be easier. I'll see if I can make something that's more generic but simple and robust.

The slice result preview "should" in theory be simple enough, just have to figure out where I need to poke in the code to make it move. I think reloading the changed gcode is the doable part, but loading back to the original does not sound like a feasible option.

On a different subject...
It would be nice if there was a ToolTip attached to each name in the "Post Processing Scripts" list.

That should also be doable, but it requires some changes to the qml, adding a getter to the python with a pyqtsignal and such, and reading the description in a try/except to handle scripts without a description.
The main issue is that the qml for that part is fairly old, so it will probably need some updating:

  • List is not properly spaced (line height / label height) -> this is mostly my OCD and other things being triggered
  • The selected highlights is not great -> same as above xD
  • The text (script name) is added to the list makes it so that the tooltip only appears if you hover over the left part (first 50 pixels or something)
  • There is also always the risk of the unexpected bug popping out of nowhere

@GregValiant
Copy link
Collaborator Author

I hate unexpected bugs. The expected ones are much better.
I think I've got the "disallowed areas" figured out without having to exclude those printers.

@GregValiant
Copy link
Collaborator Author

Wow. I have been integrating the "Disallowed Areas" into the code. There wasn't a lot of breakage, but there are more variable names to deal with (and there was already a lot).
Straightening that out is going to take a minute. There are a lot of combinations to set up for to debug.
Fortunately, not many (if any) delta's have disallowed areas so I'm ignoring it for the G2/G3's.

I'll go through your suggestions and make the changes and then push another commit.

@HellAholic
Copy link
Contributor

HellAholic commented Dec 15, 2024

It might also be an idea to use enum strings, doesn't even need to be linked to the string value it can be an int, but to keep it as close to the current implementation as possible:

from enum import Enum

# class syntax
class Location(Enum):
    LEFT_FRONT = "LF"
    LEFT_REAR = "LR"
    RIGHT_FRONT = "RF"
    RIGHT_REAR =  "RR"
    
 # usage
 if purge_end_loc == Location.LEFT_FRONT

makes it easier to work with strings in the comparisons and also reduces the chance of making a typo, also helps with autocomplete xD

You can also do the calculations for some values and store them in the self, since it's being passed along to the different functions, that will reduce the number of variables you need to pass with each function call.
curaApp is one example, but probably you'll find other things that can be done the same way.
so if in your initialize() function instead of curaApp = ... you say self.curaApp = ...
then in all the following functions that have the self as the first argument, you can just reference it by saying:
self.curaApp.getProperty("something", "something")
the only thing you need to keep in mind is not adding the same name as the super class. (make unique names)

TLDR:
Add the bits you're comfortable with and I'll do a refactor afterwards, make sure functionality is there, I'll make it pretty :P

@GregValiant
Copy link
Collaborator Author

I have to append this to my "List of things I didn't know".
image
IS A LIE.
Any gcode file (or UFP file) opened in Cura can be post-post-processed if the "Save to..." button is clicked. That extra post-processing generally makes a mess.

Plugins have a block so that doesn't happen. I've blocked that from happening with this script.

@HellAholic
Copy link
Contributor

Although it's a bit of a rare usecase to reload a gcode and run a post processing script on it, since the information regarding the post processing is already in at the end of header of the gcode file it should be possible to allow for running additional post processing without re-running the same scripts that already were executed on the gcode.
So reading the gcode, looking if there is a POSTPROCESSED comment in it, if so, filtering out the scripts already executed, and executing the other ones.

;END_OF_HEADER
;POSTPROCESSED
;  [PurgeLinesAndUnload]

@GregValiant
Copy link
Collaborator Author

In the next week or two I'm going to tear this down and put it back together. It's working, but with the changes I've hacked in for the Prime Tower move and the Disallowed Areas it's turned into spaghetti code. It's pretty bad when even I can't stand it.

So take a break. I'll add a new commit when I get done changing it.

@GregValiant
Copy link
Collaborator Author

@HellAholic
I have made alterations, added a procedure, moved things around, found a couple of typos, and altered comments.
There are a lot of changes.
Do you want me to add a commit or go about updating this some other way?

@HellAholic
Copy link
Contributor

Hey Greg,
Add the commit and I'll work based on that. Took a break and didn't do much, mostly end of year work.

Added a "quick purge" option before the actual unload to insure the filament is free to pull back.
Made adjustments for "Machine Disallowed Areas".
Added some comments.
Re-ordered some of the code.
@GregValiant
Copy link
Collaborator Author

There it is. New and improved.

One of the printers I used for the Disallowed Areas was a UM3. It has 6 areas defined and because I used a simple method of determining the "useable space" as a rectangle (rather than the actual shape), it ends up pushing the purge lines towards the print. There aren't many printers with the problem though, so the warning message I added should (hopefully) be sufficient.

@HellAholic
Copy link
Contributor

Adding a _v2 of the script to here mostly for safe-keeping, I'm just playing around with the code to see if I can simplify it a bit. Might break things so I'm doing it on a separate version to compare the output at the end.
I'll remove it from the PR once it's either at a good condition, or beyond saving :)

@HellAholic
Copy link
Contributor

HellAholic commented Dec 24, 2024

@GregValiant I'm a bit confused by the _move_to_start
example below:

            if self.start_location == "LF":
                if goto_str == "LtFrt":
                    move_str += f"G0 F{self.speed_travel} X{self.machine_right - 5} ; Ortho move\n"
                    move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
                    move_str += f"G0 F{self.speed_travel} Y{self.machine_front + 5} ; Ortho move\n"
                    move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"

we want to move to the start location, and if I understand the code, we're already there, but the moves are to right front.
When you got some time, could you take another look at that section?
Tried to make sense of the bigger picture, but I don't get the pattern as if I'm missing part of it.

@GregValiant
Copy link
Collaborator Author

There are a couple of possible options for where the "Move to Start" will originate. Since it is the last alteration of the gcode, we know where it's going TO, but it might be starting out from the Right Rear if "Add Purge Lines" is selected, or it might be starting from the Prime Tower location if that move is in the Gcode. So we don't really know the start point until other things are decided.

The design of the script was mostly done using 5.7 and 5.8. The "Layer start X" and "Layer start Y" seem to be different in 5.9.0. In earlier versions they would also dictate the start of the bed adhesion, but now the bed adhesion start seems to be related to the Z-seam location, or possibly the support Z seam location.

I'll take another look at it this evening. I did remove one of the 'If "LF" ' sections because the nozzle was definitely going to be in the left front corner. Maybe the one you noted can be taken out as well.

Since I brought up "going to..." I might have to abandon using the Layer Start X and Y in favor of reading the start of the first extrusion of the print. That way if something is affecting the skirt/brim starting point it will be taken into account.

@GregValiant
Copy link
Collaborator Author

I got side-tracked by the Start X and Y thing.
In that snippet it does appear that the first move should be to the "machine_left + 5".

if self.start_location == "LF":
if goto_str == "LtFrt":
move_str += f"G0 F{self.speed_travel} X{self.machine_left + 5} ; Ortho move\n"
move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
move_str += f"G0 F{self.speed_travel} Y{self.machine_front + 5} ; Ortho move\n"
move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"

Through the entire design stage I assumed (Hah!) that using the midpoint of the build plate to determine the "quadrant" would be correct. Last night I loaded a small model and put it in the left-front. It would seem that I should have used the model midpoint to determine the "starting quadrant".
With the model in the "LF", and the Z-seam at the "RR", if the model fits into the LF Quadrant, then the move still goes across the footprint of the model.
Now I'm thinking that the MIN and MAX of the model should be read, and the "midpoint" determined from those, rather than from the build plate size.

This is really making my head hurt. It appears that "_get_print_extents" was abandoned-in-place. I don't see it being used anywhere. The only reference is "max_print_size = self._get_build_plate_extents()" and then max_print_size isn't used.

Merry Christmas by the way.

@HellAholic
Copy link
Contributor

HellAholic commented Dec 25, 2024

Merry Christmas :)
I have a ticket planned to make the initial move an optional setting which is disabled by default and always listen to the user facing setting for Start X and Start Y, so that should make things more consistent.

So in theory, the start of the bed adhesion / skirt / support on the first layer should be the closest point to the Start X / Y, but the main issue is the printer definitions are community contributed over 7-ish years and they do not all follow the best practice. So that's why you see references to prime tower on a single extruder printer that cannot even enable prime tower. That's another challenge to standardize the definitions, and a separate subject.

For the move to start, if I boil it down until only the core remains we have this part:

        if self.bed_shape == "rectangular":
            if goto_str == "LtFrt" and self.start_location is not "LF":
                move_str += f"G0 F{self.speed_travel} X{self.machine_left + 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
                move_str += f"G0 F{self.speed_travel} Y{self.machine_front + 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
            elif goto_str == "RtFrt" and self.start_location is not "RF":
                move_str += f"G0 F{self.speed_travel} X{self.machine_right - 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
                move_str += f"G0 F{self.speed_travel} Y{self.machine_front + 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
            elif goto_str == "LtBk" and self.start_location is not "LR":
                move_str += f"G0 F{self.speed_travel} X{self.machine_left + 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
                move_str += f"G0 F{self.speed_travel} Y{self.machine_back - 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
            elif goto_str == "RtBk" and self.start_location is not "RR":
                move_str += f"G0 F{self.speed_travel} X{self.machine_right - 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
                move_str += f"G0 F{self.speed_travel} Y{self.machine_back - 5} Z2 ; Ortho move\n"
                move_str += f"G0 F600 Z0 ; Nail down the string\nG0 Z2 ; Move up\n"
        elif self.bed_shape == "elliptic" and self.origin_at_center:
            radius = self.machine_width / 2
            offset_sin = round(2 ** .5 / 2 * radius, 2)
            if goto_str == "LtFrt":
                move_str += f"G0 F{self.speed_travel} X-{offset_sin} Z2 ; Move\nG0 Y-{offset_sin} Z2 ; Move to start\n"
            elif goto_str == "LtBk":
                if self.start_location == "RR":
                    move_str += f"G2 X0 Y{offset_sin} I{offset_sin} J{offset_sin} ; Move around to start\n"
                else:
                    move_str += f"G0 F{self.speed_travel} X-{offset_sin} Z2 ; Ortho move\nG0 Y{offset_sin} Z2 ; Ortho move\n"
            elif goto_str == "RtFrt":
                move_str += f"G0 F{self.speed_travel} X{offset_sin} Z2 ; Ortho move\nG0 Y-{offset_sin} Z2 ; Ortho move\n"
            elif goto_str == "RtBk":
                move_str += f"G0 F{self.speed_travel} X{offset_sin} Z2 ; Ortho move\nG0 Y{offset_sin} Z2 ; Ortho move\n"
        move_str += ";---------------------[End of layer start travels]"

I believe you're trying to over engineer things a bit too much. So if end of purge (start_location -> should rename to end_purge_location for clarity xD) and the Start of Layer are the same, we don't do anything. Otherwise regardless of where the print head is, we just simply tell it to go to that location and don't filter the extra X/Y moves, if the print head is at X or Y coordinates already then nothing should happen, if not, then it moves there.

Also since you're calculating machine_left/right/top/bottom based on the shape of the build plate and origin, then you can reuse the same format (I double checked, should be fine). So it makes the code much simpler and boiled down.

Of course you can add more to it, but if you have a generic base, you can add functions that trim the move based on the first extrusion location.

Example of possible extensions:
For the locations I used a string enum and you're sing just strings, but we can also use a Tuple.
The good bit about the Tuple is that it's ordered and cannot be changed afterwards.
So we can use that as ("left", "front") and perform operations based on that.
If we have the end purge location at ("right", "front") and the start X/Y at ("left", "front") then we can say:

def describe_move(a, b):
    """
    Compare two tuples and describe the move from A to B.
    
    Parameters:
        a (tuple): The starting tuple (e.g., ("left", "front")).
        b (tuple): The target tuple (e.g., ("right", "front")).
    
    Returns:
        str: A description of the move from A to B.
    """
    # Validate input
    if len(a) != 2 or len(b) != 2:
        raise ValueError("Both inputs must be tuples of length 2.")
    
    # Extract components
    start_side, start_position = a
    target_side, target_position = b
    
    moves = []
    
    # Compare sides
    if start_side != target_side:
        moves.append(f"Move from {start_side} to {target_side}")
    
    # Compare positions
    if start_position != target_position:
        moves.append(f"Move from {start_position} to {target_position}")
    
    # Combine moves or indicate no movement
    return " and ".join(moves) if moves else "No movement required"

# Example usage
A = ("left", "front")
B = ("right", "front")
print(describe_move(A, B))  # Output: "Move from left to right"

Let me know if this makes sense.

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.

2 participants