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

Skipping the first deretraction #81

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

Conversation

jperry004
Copy link

@jperry004 jperry004 commented Apr 14, 2024

Description:
This pull request addresses a specific issue where a double deretraction occurs at the start of an arc movement, identified in issue #80. The change ensures that the first deretraction command before any new arc infill is skipped, under the assumption that it is automatically added by the slicer during layer changes or travel moves.

Changes Made:
Code Modification: Updated the logic to conditionally append deretraction commands based on the arc index. If arcidx (the index of the current arc segment being processed) is greater than 0, the script appends a deretraction command. This prevents the initial unwanted deretraction that occurs when the arc index is 0, likely introduced automatically by the slicer.

@jperry004
Copy link
Author

jperry004 commented Apr 15, 2024

We also need to skip the very last retraction of an arc infill as well, because going back to slicer generated gcode won't have the associated deretraction. Or perhaps add the needed deretraction after the next movement.

image

Skip the final arc retraction because the associated deretraction will not be generated by the slicer.
@jperry004
Copy link
Author

I came up with a fix for the final retraction, although it's a bit more complicated than I wanted. Let me know if you see a simpler way.

@Wasupmacuz
Copy link

Let me know if you see a simpler way.

Testing one right now, I'll submit a pull req to your branch if it works.

Copy link

@Wasupmacuz Wasupmacuz left a comment

Choose a reason for hiding this comment

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

Apparently, forking a fork is way more effort than it's worth. I'll try to explain as best I can here, referencing your first commit.

First change

Taking the retraction from the last arc away:

  • Move line 1167 (GCodeLines.append(retractGCode(retract=True,kwargs=kwargs))) to line 1153.
if idp==0:
    GCodeLines.append(retractGCode(retract=True,kwargs=kwargs)) # Retract before starting this next arc. We don't retract after the last arc!
    p1=p

This changes The GCODE at the end of the final arc to something we like:
image
However, now we're retracting before we start the first move. We've moved the issue from the end of arc generation to the beginning:
image

Second change

Taking a look at the conditional:

  1. Adding the same conditional that you have to disable the first deretraction (extrusion?) could be added to this pre-arc retraction.
if idp==0:
    if arcidx > 0:
        GCodeLines.append(retractGCode(retract=True,kwargs=kwargs)) # Retract before starting this next arc. We don't retract after the last arc!
    p1=p

This gets rid of the extra retraction at the beginning:
image
But wouldn't it be nice if we had a retraction and extrusion before and after (respectively) the highlighted travel move? Luckily, we can do this super easily!
2. Remove both the conditionals from our retraction and extrusion.

def arc2GCode(arcline:LineString,eStepsPerMM:float,arcidx=None,kwargs={})->list:
    GCodeLines=[]
    p1=None
    pts=[Point(p) for p in arcline.coords]
    if len(pts)<2:
        return []
    #plt.plot([p.x for p in pts],[p.y for p in pts])
    #plt.axis('square')
    #plt.show()
    extDist=kwargs.get("ExtendArcDist",0.5)
    pExtend=move_toward_point(pts[-2],pts[-1],extDist)
    arcPrintSpeed=np.clip(arcline.length/(kwargs.get("ArcSlowDownBelowThisDuration",3))*60,
                            kwargs.get("ArcMinPrintSpeed",1*60),kwargs.get('ArcPrintSpeed',2*60)) # *60 bc unit conversion:mm/s=>mm/min
    for idp,p in enumerate(pts):
        if idp==0:
            GCodeLines.append(retractGCode(retract=True,kwargs=kwargs)) # Retract before starting this next arc. We don't retract after the last arc!
            p1=p
            GCodeLines.append(f";Arc {arcidx if arcidx else ' '} Length:{arcline.length}\n")
            GCodeLines.append(p2GCode(p,F=kwargs.get('ArcTravelFeedRate',100*60)))#feedrate is mm/min...
            GCodeLines.append(retractGCode(retract=False,kwargs=kwargs))
            GCodeLines.append(setFeedRateGCode(arcPrintSpeed))
        else:
            dist=p.distance(p1)
            if dist>kwargs.get("GCodeArcPtMinDist",0.1):
                GCodeLines.append(p2GCode(p,E=dist*eStepsPerMM))
                p1=p
        if idp==len(pts)-1:
            GCodeLines.append(p2GCode(pExtend,E=extDist*eStepsPerMM))#extend arc tangentially for better bonding between arcs
    return GCodeLines

This is the final code. It avoids added complexity, requires no extra conditionals from Nic's code, and moves only one line.

Beginning of arcs:

image

Arcs in the middle:

image

Final arc:

image
I'd like to note that if you look at that highlighted line in the final arc GCODE, that's our restored, pre-injected tool position. I noticed that the extrusion value is not zero. This will cause stringing if it moves over a hole or gap. This should be reported as an issue and the restored tool position's E value should be set to zero in a future script patch.

@jperry004
Copy link
Author

jperry004 commented Apr 16, 2024

Nice work! I don't really understand the change tbh, but it does work around the arcs. However now I'm seeing 1 extra deretract about 150 lines later.

image

Checking with grep we find 25 deretractions added by the script, but there's only 24 arcs in this one.
image

I'll add the two gcode files here, I had to add .txt for github to accept the files.

original-test.gcode.txt
original-test-arcs-added.gcode.txt

Edit - Since this part of the code only adds the detraction after

GCodeLines.append(f";Arc {arcidx if arcidx else ' '} Length:{arcline.length}\n")
then the bad deretraction must be added as part of the hilbert infill section for the next layers. I'm thinking this solution is good to go.

@Wasupmacuz
Copy link

Wasupmacuz commented Apr 16, 2024

I'm seeing 1 extra deretract about 150 lines later.

Do you have "specialCoolingZdist" set to a negative value? It looks like this may actually by related to #66. I never looked at the GCODE when I suggested setting it negative to disable Hilbert curves, I only looked at the sliced view.

Some simple logic needs to be added to disable hilbert curves.

Was possibly correct? I need to look into this to know what's really happening because I am so lost.

@jperry004
Copy link
Author

I'm seeing 1 extra deretract about 150 lines later.

Do you have "specialCoolingZdist" set to a negative value? It looks like this may actually by related to #66. I never looked at the GCODE when I suggested setting it negative to disable Hilbert curves, I only looked at the sliced view.

Some simple logic needs to be added to disable hilbert curves.

Was possibly correct? I need to look into this to know what's really happening because I am so lost.

I was testing it with stock settings. But I just tested it with the negative value and the extra one is gone. This fix is great, nice work. I want to squash down the reverts before merging, but I think we're all set here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants