Skip to content

Comments

Fix for Fwretract bugfix1.1.x #9872

Merged
thinkyhead merged 4 commits intobugfix-1.1.xfrom
unknown repository
Mar 1, 2018
Merged

Fix for Fwretract bugfix1.1.x #9872
thinkyhead merged 4 commits intobugfix-1.1.xfrom
unknown repository

Conversation

@ghost
Copy link

@ghost ghost commented Mar 1, 2018

  • The BUG is not on 2.0 , just on 1.1.x / bugfix ' see the issue explanation 9812'
  • The line in G28 doesn't reset FwRetract , because of Hopamount that is keep alive and that will be added on the first G11 and go down double

The only possibility is to make a comment in config.adv to inform Fwretract don't resetted with G28

Or to rewritte Fwretract to give possibilit of reset by G28
But that's not an issue , if someone make a g28 after G10S1 , it's a user fault , not a marlin issue

What do you think ? We write a reset code for Fw or we merge my PR

@thinkyhead
Copy link
Member

thinkyhead commented Mar 1, 2018

We write a reset code for Fw or we merge my PR

I'll merge this because it makes the code more correct and gives parity with 2.0.x.

But also, let's make sure that auto-retract is turned off during (and restored after) filament change. I think that makes good sense.

We could have G28 reset the firmware retract variables as a precaution, but I don't think it's vital.

@thinkyhead thinkyhead merged commit d25f0a5 into MarlinFirmware:bugfix-1.1.x Mar 1, 2018
@tcm0116
Copy link
Contributor

tcm0116 commented Mar 4, 2018

I disagree with this PR. The way Z hop is implemented, if G28 is executed after a G10, on the next G11, the Z height will be reduced by the Z hop amount. Let me give an example.

  • Z = 10, Z hop = 1
  • Send G10
  • Z is still reported as 10, but is actually at 11
  • Send G28
  • Send G1 Z0
  • Send G10 - nothing happens
  • Send G11 - nozzle now moves 1 mm below the bed

The Z hop must be reset when performing G28, otherwise the Z position can get out of sync. Yes, G10 followed by G28 is not ideal, but it can happen, which could result in damage to the bed. I know this can happen because I have a nice gouge in my print bed. This is precisely why #9390 and #9384 were created. If we want to provide a different solution, that's fine, but Marlin now has the ability to damage printers when using firmware retraction.

@thinkyhead
Copy link
Member

Phew! If we can't trust the experts who can we trust? 😝

Do you think it should clear on any G28 or just any G28 with Z?

Can you think of other conditions where it would be sensible to reset it?

@tcm0116
Copy link
Contributor

tcm0116 commented Mar 4, 2018

Per our discussion in #9390, it should only be cleared when homing the Z axis. I can't think of any other conditions that completely reset the position of the Z axis relative to the endstops.

@thinkyhead
Copy link
Member

Right! Restored at bdfeb54 and f3914a4.

@ghost
Copy link
Author

ghost commented Mar 4, 2018

I love the idea that G28 makes a total cleaning of all possible. Really I do, balls to bones. And I'm sure every user that makes a G28 thinks it's a machine reset .... I certainly do.

But only a G28 that includes Z homing, naturally.

  • Keep the original status reset in G28
  • Add a line in FWRETRACT management like this:
    if (retracted[active_extruder] && !retracted_swap[active_extruder]) hop_amount = 0;
    This line will reset hop_amount if hop_amount abides without retracted status.
  • Add a comment in FWRETRACT "Reset statuses and zlift when Z homing"

Or My Favourite

-Rewrite Fw_retract::reset() function , and add hop_amount as global var
-Rewrite G28 FwReset code to call Fw_retract::reset()
-Add a comment in FWRETRACT "Z Homing reset current statuses & zlift"

hop_amount is static , then , static or global , there's no difference in sram consumption, then we can make a clean code in this second way

@thinkyhead If you want I write it , make my tool_migration feature in your own name , and i will fork a new marlin , my fork is too old

@thinkyhead
Copy link
Member

@studiodyne — Both options combined, perhaps.

@thinkyhead
Copy link
Member

i will fork a new marlin , my fork is too old

I urge anyone who wants to use Github regularly, to maybe (just maybe) learn how to use git.

thinkyhead pushed a commit that referenced this pull request Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants