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

Latest RCBugFix Homing weirdness #4325

Closed
meros opened this issue Jul 16, 2016 · 26 comments
Closed

Latest RCBugFix Homing weirdness #4325

meros opened this issue Jul 16, 2016 · 26 comments
Labels
Bug: Confirmed ! Needs: Patch A patch is needed to fix this
Milestone

Comments

@meros
Copy link

meros commented Jul 16, 2016

Using a i3 style printer homing behaviour has changed drastically last few days. It used to go X->0, then Y->0, then Z->0. Now it drives X and Z at same time, then Y, then Z again. After homing is done it goes to X=90, Y=90, Z=0 (also new, it used to stay put).

This ruined a print due to G28 in the end of the print caused the head to crash into printed plastic.

My configuration is here: meros@a60ef94

@Roxy-3D
Copy link
Member

Roxy-3D commented Jul 16, 2016

Moving Z to 0.0 is dangerous with a print on the bed. That must be in your Slicer's Ending GCode because that is not a default behavior. If there is any miscalculation, you are going to drive the nozzle into the newly printed part.

@jbrazio
Copy link
Contributor

jbrazio commented Jul 16, 2016

Let's be pragmatic, changing the Z-pos with melted plastic laying out on the bed is not a smart move. Look at the following g-code:

; Fast move
G91
G0 X10 F15000

; Lift Z
G0 Z5.0 F120
G90

; Park
G0 X5 Y295 F9000

It uses relative coordinated to lift Z a bit and them move to a park position somewhere on the bed. When #4299 is merged you just have to replace all that gcode with a simple G27 P2.

@meros
Copy link
Author

meros commented Jul 17, 2016

I am using Cura, nothing exotic. I am probably wrong in that it used to go Z=0 when done, it makes sense that it should never do that, even at X=0, Y=0.

This is the ending sequence (as generated by cura):

M107
M104 S0 ;extruder heater off
M140 S0 ;heated bed heater off (if you have it)
G91 ;relative positioning
G1 E-1 F300  ;retract the filament a bit before lifting the nozzle, to release some of the pressure
G1 Z+0.5 E-5 X-20 Y-20 F9000 ;move Z up a bit and retract filament even more
G28 X0 Y0 ;move X/Y to min endstops, so the head is out of the way
M84 ;steppers off
G90 ;absolute positioning
M104 S0.000

There are two problems here, first and most importantly, Z moves when print is done. This ruins the print. Secondly, the behaviour when doing a normal auto home is also weird. I think both these are due to G28 (just different parameters)?

I have this behaviour on RCBugFix as it was late yesterday.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 17, 2016

@meros Apologies! A lot of changes have gone into homing/leveling and these changes do need more review. I have been unable to do as much testing on my own as I would like, and have been mostly relying on feedback.

Are you using an ordinary Z min endstop for homing, and no Z probe on your machine? (I speculate that perhaps some probe-deploy move is happening here but in the wrong instance…)

Are you using SAFE_HOMING at all? Again, I want to make sure no odd moves are occurring.

Some new choices in movement functions for G28 have happened lately, and these probably need some more careful scrutiny. Movement functions previously related only to G29 actions might be involved.

@thinkyhead thinkyhead added Bug: Potential ? Needs: More Data We need more data in order to proceed labels Jul 17, 2016
@meros
Copy link
Author

meros commented Jul 17, 2016

I have no probe, only z min endstop. I use bed mesh leveling, perhaps that is part of the problem? G28 in combination with bed mesh leveling could be an edge case? That leveling normally works really good though :)

I have tried with and without SAFE_HOMING with the same result.

@thinkyhead
Copy link
Member

@meros What do you have set as your MIN_Z_HEIGHT_FOR_HOMING ?

@meros
Copy link
Author

meros commented Jul 17, 2016

It's not explicitly set:

// @section homing
//#define MIN_Z_HEIGHT_FOR_HOMING 4 // (in mm) Minimal z height before homing (G28) for Z clearance above the bed, clamps, ...
// Be sure you have this distance over your Z_MAX_POS in case.

@jbrazio jbrazio added Bug: Confirmed ! Needs: Patch A patch is needed to fix this and removed Bug: Potential ? Needs: More Data We need more data in order to proceed labels Jul 17, 2016
@jbrazio
Copy link
Contributor

jbrazio commented Jul 17, 2016

There is indeed a bug with G28.
I did the following:

G28
G28 X0
G28 Y0
G28
G28 X1
G28 Y1
G28
G28 X1Y1 ; from now on each time I issue a individual axis home
         ; the Z axis goes lower into the bed despite MIN_Z_HEIGHT_FOR_HOMING being 5
         ; even if I try G28 alone Z will dig deeper into the bed

@thinkyhead
Copy link
Member

Enable DEBUG_LEVELING_FEATURE and use M111 S32 to enable extra logging. Do something that invokes this weird behavior and post the log output here so we can have a look and see what might be going on. I've just added more logging, so be sure to get the latest RCBugFix code first.

@thinkyhead
Copy link
Member

G28 X0 and G28 X1 are the same thing, btw. The G28 command just looks for the letter X.

@jbrazio
Copy link
Contributor

jbrazio commented Jul 17, 2016

Yes I know, but I was testing for errors on the parser.

@thinkyhead
Copy link
Member

I suspect if we revert G28 to before the recent change to use do_blocking_move functions, it will behave better.

@jbrazio
Copy link
Contributor

jbrazio commented Jul 17, 2016

Reverting to the previous style makes no sense if we already decided to start using them as the standard, I suppose we shall start a daemon* hunting session, get your pitchforks !

* aka bug.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 17, 2016

I suggest it as something to try. Experimentation to narrow down the cause. In other words, I don't feel like reading through the code hoping the bug will just jump out at me.

@AnHardt
Copy link
Member

AnHardt commented Jul 17, 2016

Hmmm. :-(
I see.
We have the crazy things since

       if (home_all_axis || homeX || homeY) {
          // Raise Z before homing any other axes and z is not already high enough (never lower z)
 -        float z_dest = home_offset[Z_AXIS] + MIN_Z_HEIGHT_FOR_HOMING;
 -        if (z_dest > current_position[Z_AXIS]) {
 +        destination[Z_AXIS] = home_offset[Z_AXIS] + MIN_Z_HEIGHT_FOR_HOMING;
 +        if (destination[Z_AXIS] > current_position[Z_AXIS]) {

            #if ENABLED(DEBUG_LEVELING_FEATURE)
              if (DEBUGGING(LEVELING)) {
 -              SERIAL_ECHOPAIR("Raise Z (before homing) to ", z_dest);
 +              SERIAL_ECHOPAIR("Raise Z (before homing) to ", destination[Z_AXIS]);
                SERIAL_EOL;
              }
            #endif

 -          feedrate = homing_feedrate[Z_AXIS];
 -          line_to_z(z_dest);
 -          stepper.synchronize();
 -          destination[Z_AXIS] = current_position[Z_AXIS] = z_dest;
 +          do_blocking_move_to_z(destination[Z_AXIS]);
          }
        }

The reason is the less and less use off destination so homeaxis (https://github.com/MarlinFirmware/Marlin/blob/RCBugFix/Marlin/Marlin_main.cpp#L2381), what heavily uses

  destination[axis] = 1.5 * max_length(axis) * axis_home_dir;
  feedrate_mm_m = homing_feedrate_mm_m[axis];
  line_to_destination();
  stepper.synchronize();

gets somewhat random historic values at the other (not homed) axis.

By investigating this I discovered that do_blocking_move_to() currently sets destination to current_position only for DELTA.

I'm close to a fix. Needs more testing. PR likely tomorrow.

@thinkyhead
Copy link
Member

thinkyhead commented Jul 17, 2016

Don't rely on destination for anything except as a way to prepare coordinates for one of the _to_destination functions. It was never meant to keep a running position throughout these procedures, and it may be overwritten by anything that wants to use it. For keeping track of the current position, I humbly suggest using current_position — then rely on those functions that explicitly set it, or which copy destination into it.

@AnHardt
Copy link
Member

AnHardt commented Jul 17, 2016

Preview at AnHardt#58

@jbrazio
Copy link
Contributor

jbrazio commented Jul 18, 2016

Will test when I arrive home later today.

@meros
Copy link
Author

meros commented Jul 18, 2016

G28 seems to work as expected now for me.

@Roxy-3D
Copy link
Member

Roxy-3D commented Jul 18, 2016

When #4299 is merged you just have to replace all that gcode with a simple G27 P2.

@jbrazio This is a typo, right? I have a new Delta Physical Bed Leveling command at G27.

@jbrazio
Copy link
Contributor

jbrazio commented Jul 18, 2016

@Roxy-3D not a typo.. G27 is now nozzle parking on RC7. 😢

-edit- I raised question #4346 which may be relevant for your UBL work.

@Roxy-3D
Copy link
Member

Roxy-3D commented Jul 18, 2016

not a typo.. G27 is now nozzle parking on RC7. 😢

@jbrazio @thinkyhead I'll move my G27 to G25. (G26 is already defined as the Mesh Validation Command) Please don't assign G24 to anything.

@jbrazio
Copy link
Contributor

jbrazio commented Jul 18, 2016

@Roxy-3D which g-codes and their descriptions/function have you added for UBL ?

@Roxy-3D
Copy link
Member

Roxy-3D commented Jul 18, 2016

M43 GPIO pin scan (to help identify logical pin locations of expansion header with physical location)
G25 Delta Physical Bed Leveling tool (Still at G27 in my code... But I'll move it)
G26 Mesh Validation Tool for UBL
G24 Please Reserve for UBL future use!

@jbrazio jbrazio modified the milestone: 1.1.0 Jul 22, 2016
@thinkyhead
Copy link
Member

@meros More adjustments have been made to coordinate handling in the last several days. Please give RCBugFix another try and see if the issue persists. We definitely want to try and fix this ahead of release.

@github-actions
Copy link

github-actions bot commented Apr 5, 2022

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug: Confirmed ! Needs: Patch A patch is needed to fix this
Projects
None yet
Development

No branches or pull requests

5 participants