Some more probe cleanup

[AnHardt]

-    if (probe_action & ProbeStow) {
+    if (stow) {
       stow_z_probe();
     }
+    else {
+      do_probe_raise(max(current_position[Z_AXIS],Z_RAISE_BETWEEN_PROBINGS));
+    }

is the key idea in probe_pt().
The rest is more or less the concequences.

And a simple servo tweak to prevent the probe from moving before the servo is at its target angle.

Not very well tested.

Best viewd with https://github.com/AnHardt/Marlin/pull/48/files?diff=unified?w=1

[thinkyhead]

Looks like a reasonable simplification. The probe_pt function was a little "overdesigned."

I found a cool trick… It turns out I can create a PR from any fork using the usual form…

https://github.com/MarlinFirmware/Marlin/compare/RCBugFix...AnHardt:more-probe-simplifications?expand=1

[thinkyhead]

Aha, I see…
If z is higher, then raise to z before moving XY.
If z is lower then move down to z only after moving to XY.
Interesting…

[AnHardt]

This was intentional to have enough height to move in x/y direction before the probe is deployed. like announced in the debug message above. If needed the raise while deploying the probe is separate.
The feedrate is really not needed. It's always overruled in do_blocking_move()

[thinkyhead]

That's fine and makes sense. But it should use do_probe_raise(Z_RAISE_BETWEEN_PROBINGS) instead, because the lift here needs to account for home_offset[Z_AXIS] and the negative probe offset.

[AnHardt]

However. If we want to travel here we have to be high enough. This should be done here - not in every callers.

[thinkyhead]

Makes sense! We probably ought to do a sanity check to make sure that

   Z_RAISE_BETWEEN_PROBINGS <= Z_RAISE_BEFORE_PROBING
&& Z_RAISE_BETWEEN_PROBINGS <= Z_RAISE_AFTER_PROBING

It's probably also possible to rename these if we wanted:

Z_RAISE_BEFORE_PROBING => Z_RAISE_PROBE_DEPLOY
Z_RAISE_AFTER_PROBING  => Z_RAISE_PROBE_STOW

Or, heck, they could both be combined into one setting really…

Z_RAISE_PROBE_DEPLOY_STOW

[AnHardt]

Combining is a good idea - also for Z_RAISE_BETWEEN_PROBINGS == MIN_Z_HEIGHT_FOR_HOMING

How about something like

#define _Z_RAISE_PROBE_DEPLOY_STOW  max(Z_RAISE_PROBE_DEPLOY_STOW, Z_RAISE_BETWEEN_PROBINGS)

instead of a error message - in Conditionals.h.

I'd like to se MIN_HEIGHT_* instead of Z_RAISE_*

[thinkyhead]

Some machines, mostly deltas, have either a large raise-before or a large raise-after value. Do you think those are important? For any machine that homes to Z_MAX the probe will deploy before it moves down at all. So I believe a delta that wants to deploy at Z=100 will behave the same as one that wants to deploy at Z=10. It will just deploy at the top. (We could change that by having do_probe_raise lower the nozzle in such cases.) But three deltas (kossel-mini, generic, biv2.5) have a raise-after of 50mm. I doubt that is so important, but why 50mm I wonder?

I will set them to the higher value, see if that works for most deltas….

MarlinFirmware#4153

[AnHardt]

Some machines, mostly deltas, have either a large raise-before or a large raise-after value.

As far as i know this is/was because of the bowl shape moves. They want to be far away from the bed when deploying/stowing. If you have a cloth look on the mechanics if the Allen key probes you will discover - the deploy move could be purely horizontal. The extender of the short side of the Allen key has to hit the belt or tower to turn the key from its rest. That does not need a vertical move. Stowing is different. At a special x/y place there the move is vertical downward until the spring can turn the key on its rest.
If we don't have the bowl shaped moves any more the very high raises are not needed. The raise shold be high enough to bring the pit of the Allen key above the stow platform. Oterwise, (we still use potentially diagonal moves) the tip could collide with the platform if the start point is cloth to the platform.

Ok. From the beginning.
For the deltas, an all other machines homing to z-max, we have not to deal with unhomed axis for deploying/stowing. We don't need a z-min-probe for homing to max.
As soon z is homed to max (we are still at the top) we know we are very high compared to our raise/min-height values - so we know we do not need to rise.
A raise move would be disastrous.
A unconditional move to a special high will probably be to deep. The current deploy height for the allen keys is at about the mid of the usable height of the towers.
A conditional move (like we do now) seems to be perfect.

The Allen key moves need special attention. Right after homing we are still at the very top. Here we can not move in x/y-direction before we are a bit deeper - can not use do_blocking_move().
At the low end of the z axis the blocking moves are welcome.

I see a problem with some of the default values for the Allen key moves. For example "mini kossel". Here we can find:

  // Kossel Mini
  #define Z_PROBE_ALLEN_KEY_DEPLOY_1_X 30.0
  #define Z_PROBE_ALLEN_KEY_DEPLOY_1_Y DELTA_PRINTABLE_RADIUS
  #define Z_PROBE_ALLEN_KEY_DEPLOY_1_Z 100.0
  #define Z_PROBE_ALLEN_KEY_DEPLOY_1_FEEDRATE XY_PROBE_SPEED

If DELTA_PRINTABLE_RADIUS is indeed the limit then the point can't be reached. a² + b² > a². The same schuld be true for the stow-platform-point. Ideally this should be outside the normal print area. Otherwise a collision while printing is likely. With working software endstops it will become a problem to reach this points.
The same i realized for the sled probe. As soon a positive SLED_DOCKING_OFFSET is defined the position can not be reached. For these moves we'd have to disable the software endstops.
Also 'cleaning area' should be protected but reachable.

Sorry for the monologue - you solved the problem by your self - an i agree.