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

Divide Z and XY moves in do_blocking_move_to for Delta #4363

Closed

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Jul 20, 2016

Background: XYZ Delta moves with do_blocking_move_to are combined. Moves like this don't work on a homed delta because there's limited XY movement at the top.

Solution: For homing and leveling moves (do_blocking_move_to) this change makes sure the Z movement is always done before XY movement.

Caveats: What about printing? When starting a print, if the first move is XYZ and the delta is homed to the top, will the XYZ end up off? It depends on how the delta ABC position is currently constrained.

References: #4361, #4356

@Blue-Marlin
Copy link
Contributor

An excellent move - if you want to engrave the bed with the nozzle.

@ghost
Copy link

ghost commented Jul 20, 2016

engrave the bed with the nozzle.

I saw it at G29.
And Z coordinate is wrong.

Video clip:

A branch that it used for test:
https://github.com/esenapaj/Marlin/tree/testes

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 20, 2016

An excellent move - if you want to engrave the bed with the nozzle.
I saw it.

Please explain. I am not fully attending to this today!

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 21, 2016

If you move from [i,j,k] to [l,m,0] you'll get a scratch from [i,j,0] to [l,m,0].

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 21, 2016

Context? I'm not fully attending to this, so please don't make me think.

How did the nozzle get to [0,0,n]? Is this a G29 move down to the probe height or a move at the end of G28? Why is there a move to [l,m,0]? Is it trying to move to the first probe point? I don't understand the context in which a move to Z=0 would occur. Is that the bug? Or is that a probe of the bed with the nozzle as the probe? The Z is wrong? It isn't raising to the before-probe height when moving to the first point?

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 21, 2016

The actual 'probe'-moves go far below zero. That works only if they are straight down. We do everything in the upper functions (probe_pt(), G29) to make them straight down - but for a moment this morning it looked like there are diagonal moves - and indeed they were there, but not because we prepared them wrong, but because the coordinate system shifted in a wrongly prepared SYNC_PLAN_POSITION_KINEMATIC.
But at the moment i'm not entirely sure if we didn't shift all the 'straight down magic' into do_blocking_move_to().

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 21, 2016

What are the implications for this particular PR? I'm having a hard time figuring out what's good, what's bad, based on comments across these connected issues.

@ghost
Copy link

ghost commented Jul 21, 2016

I've tested the updated this PR (a5cadff + b6a2b94) just now.
Broken behavior of G29 is still exactly the same as previous one,
And unfortunately, now even G28 is broken.
A downward behavior after G28 that it was introduced by PR #4303 (DELTA: Move out of the clip-zone after G28) doesn't works physically (vertical carriages is touching endstop),
but it works logically (variable of Z coordinate decreased).

@thinkyhead
Copy link
Member Author

@esenapaj Yeah, I didn't expect any improvement in this PR yet. The various comments from @Blue-Marlin are obviously very detailed and helpful. But I've been too scatter-brained the last two days to collate all the things pointed out and make the needed changes. Obviously I don't want to revert back to old code (and that would be very hard at this point) and I think we are very close to solving all the quirks. With the proposed changes, almost all moves will now update current_position and I think we will have the position-after-probing solved as well.

So first thing is, un-scatter my brain! Later today I will try to incorporate everything that we have discussed and solved.

@thinkyhead
Copy link
Member Author

straight down magic

Is that good straight down magic, or bad straight down magic? 😕

@Blue-Marlin
Copy link
Contributor

When probing, with a delta and the z-difference method, a straight down move is absolutely required. If moving in x or y the result will be wrong. See calculate_delta() (https://github.com/MarlinFirmware/Marlin/blob/RC/Marlin/Marlin_main.cpp#L7128)
So up to now good 'straight down_magic'.
With the implementation of the forwardKinematic() this strict rule can be relaxed, but simplifies to imagine what is going on. At least the straight moves do not hurt.
Still good 'straight down_magic'.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 21, 2016

So… Is it safe to merge this PR, or should we use the code that deals a little differently with the Z moves (as in this comment), or the code as you originally wrote it before I started messing with it?

@AnHardt
Copy link
Member

AnHardt commented Jul 22, 2016

I hope we agree this (#4363) is the worst.
For the lower (save) zone is best, what we do with the cartesians. Doing the (interpolated) xy-moves at the highest possible level, to not scratch the bed.
If we already are in the upper zone and the target is in the lower zone, it makes sense to leave the upper zone first, then make the xy-move and drop to the target then.
If start- and end-point are in the upper area, there are lines possible, crossing the forbidden area, when using the interpolated moves. Even if start and end points are in the allowed volume. But (this insight is new) if using the not interpolated, bowl shaped, moves we can not cross the forbidden volume, as long as start and end are allowed.
This moves are not used during printing. All this probing/leveling is done best with an empty bed. So if there is a horizontal (then hanging) line it will probably not touch a print. In homing the way is straight upward anyway - for a delta.
So using the direct, diagonal, not interpolated moves, in the upper zone seems to be best.

I'll pimp up my #4361 tomorrow - or - if you are in a hurry - you know what and how to do.

EDITED
An additional aspect is - it's only one move. If stopped by a hardware endstop - it is stopped. No additional steps are added by the remaining sub-moves.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 22, 2016

I hope we agree this (#4363) is the worst.

Well I hadn't really decided. If we only use the blocking move functions for probing and homing moves, the caller should already be using these functions in a reasonable way. In other words, using these functions with the understanding that Deltas will always do the Z move first (assuming this PR).

So let's say you start a G29 from any arbitrary position. The first move will be to the pre-probing (deploy) height in Z. That should be okay from anywhere, correct? (As long as the deploy height is in the safe zone.)

Also, on Delta perhaps it should always move to X=0, Y=0 at the same time. (Moves towards the center on a Delta are always safe because none of the carriages go above the currently-highest one.) The next move before probing could (or should) be to Z at the "between" height (if the deploy height is very high up), and then to the first probe XY position. (This could be a combined diagonal move also. See below.)

To sum up, the following G29 procedure should be safe from anywhere…

Delta probing:

  • Move to (0, 0, Z) … (center at the current height)
  • Move to probe deploy Z height (still in the center)
    • (this could even be at the very top)
  • Deploy the probe
  • Move to Z below the "danger zone" if needed
  • Move to XY of the first probe point (and the between-Z?)
  • First probe starts from the deploy Z (or safer) height
    • Do fast probe, raise to between height, do slow probe
    • Raise Z to between height (and XY?)
    • Move to the next XY (if any)
    • Keep probing until done
  • Move XY to (0, 0, Z) (center at the current height)
  • Raise to deploy Z height (or combine with XY move)
  • Stow the probe

It should also be fine to do diagonal moves between probe points. In fact, whenever moving up, XYZ can be combined to save time. Probe-down moves should always be Z-only, but not all downward moves need to be. A downward move that starts above the safe zone could move in Z to the top of the safe zone first, then move diagonally below that.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 22, 2016

I'll pimp up my #4361 tomorrow

Go for it. Consider the ideas from my previous post. It would be nice if the procedure did diagonal moves where possible. I guess with the E flag (stowing and deploying after each probe point) diagonal moves would be strange, so in that case we'd do the usual raise Z, stow, move XY, deploy…

@AnHardt
Copy link
Member

AnHardt commented Jul 22, 2016

Thinking about a special DELTA-probing sequence is an error. We need a sequence to fit all systems!

  • Move to 0, 0, current[Z] // ok? for what?
  • Move to probe deploy Z height // really? Lowering? That could be undefined or zero.
  • Deploy the probe // That's essential! Deploying/stowing the pro can exactly do the rise in the line before when needed. (And currently does)
  • Move to XY of the first probe point
  • First probe starts from the deploy Z height // right
    • Do fast probe, raise to between height, // why so high
    • Do slow probe // right
    • Raise Z to between height // right
  • Move XY to the next point, repeat until done // except we have an 'E' parameter
  • Move XY to 0, 0 after the last probe // why?
  • Raise to deploy Z height // integrated in // deploy/stow

Considering this means a complete rethinking and rewrite - again. No!

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 22, 2016

Thinking about a special DELTA-probing sequence is an error. We need a sequence to fit all systems!

Says you.

  • Move to 0, 0, current[Z] // ok? for what?

Because the center is the safest point.

  • Move to probe deploy Z height // really? Lowering? That could be undefined or zero.

I refer to G29. Assume it's a reasonable value. The machine has been homed.

  • Deploy the probe // That's essential! Deploying/stowing the pro can exactly do the rise in the line before when needed. (And currently does)

Simply noting the point in the sequence…

  • Move to XY of the first probe point
  • First probe starts from the deploy Z height // right
    • Do fast probe, raise to between height, // why so high

Inductive probes require this height to ensure they are no longer triggered.

  • Do slow probe // right
  • Raise Z to between height // right
    • Move XY to the next point, repeat until done // except we have an 'E' parameter

Assume I know that and am simply not being exhaustive here.

  • Move XY to 0, 0 after the last probe // why?

If the deploy height is at the very top then this is safe.

  • Raise to deploy Z height // integrated in // deploy/stow

One size fits all, until you consider how Delta could be slightly better.

Considering this means a complete rethinking and rewrite - again. No!

Not necessarily. But I understand your trepidation.

@AnHardt
Copy link
Member

AnHardt commented Jul 22, 2016

Inductive probes use this height to ensure they are no longer triggered.

No. between needs to be much higher. High enough to not trigger above the highest possible obstacle. While bump_high is exactly what you describe.

I fear we will never agree about that.
I'll leave that complex now - for a while.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 22, 2016

between needs to be much higher

This (Z_RAISE_BETWEEN_PROBINGS) is user-configured, correct? Specifically to give a reasonable (and fairly minimal) height for just-before-probing, correct? And for an inductive probe this must be set high enough not to trigger the probe, correct? So why require a separate probe-bump-distance at all?

@AnHardt
Copy link
Member

AnHardt commented Jul 22, 2016

To make the way short for the potentially very slow slow probe move.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jul 22, 2016

To make the way short

Why would Z_RAISE_BETWEEN_PROBINGS be set at any value larger than needed to (merely) disengage the probe?

@thinkyhead thinkyhead closed this Jul 22, 2016
@thinkyhead thinkyhead deleted the rc_analyze_G28_G29 branch July 22, 2016 03:05
@jbrazio jbrazio added this to the 1.1.0 milestone Jul 22, 2016
@jbrazio jbrazio added this to the 1.1.0 milestone Jul 22, 2016
drewmoseley pushed a commit to drewmoseley/Marlin that referenced this pull request Feb 10, 2024
…nu_shown

Fix Calibration menu shown during resuming
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.

4 participants