Marlin: Z-Probe Servo Endstop issue

[Roxy-3D]

We have this code block in Marlin_main.cpp. The problem is, if you are using a Z-Probe on a servo as your Z-Min endstop and you turn Bed Leveling off... The void raise_z_for_servo(float z_dest) function is still needed, but it is undefined.

  #if HAS_Z_SERVO_ENDSTOP

    /**
     * Raise Z to a minimum height to make room for a servo to move
     *
     * zprobe_zoffset: Negative of the Z height where the probe engages
     *         z_dest: The before / after probing raise distance
     *
     * The zprobe_zoffset is negative for a switch below the nozzle, so
     * multiply by Z_HOME_DIR (-1) to move enough away from the bed.
     */
    void raise_z_for_servo(float z_dest) {
      z_dest += home_offset[Z_AXIS];

      if ((Z_HOME_DIR) < 0 && zprobe_zoffset < 0)
        z_dest -= zprobe_zoffset;

      if (z_dest > current_position[Z_AXIS])
        do_blocking_move_to_z(z_dest); // also updates current_position
    }

  #endif

#endif // AUTO_BED_LEVELING_FEATURE

[Roxy-3D]

And actually... The problem is worse than that... The do_blocking_move-xx() routines are also inside of a:

  #if ENABLED(AUTO_BED_LEVELING)

so the raise_z_for_servo() can't call them even if it is moved outside of the #ifdef for AUTO_BED_LEVELING.

[AnHardt]

It just needs some more shifts. It's exceptional hard to find a working point before several hundred lines are shifted. But - there is a point it will work again. Been there - done that. But as a single commit this would never be accepted because it's simply too huge. On a that fast moving target like Marlin_main you need all threads in your hand to be successful.

@thinkyhead
Move on. There is no way back.

[lrpirlet]

I went around all those issue in my branch https://github.com/lrpirlet/Marlin/tree/MMBL_1_for_lrp... At least it compliles... I have not tested it yet (no time)... I think I even have the Z_MIN_PROBE_REPEATABILITY_TEST working outside the ABL... Have a look it you want.

[thinkyhead]

Adding more adjustments, a bit at a time. I didn't want to dump all of #4021 in at once.

[thinkyhead]

Have another look at #4021 and the resulting code for G28, G29, G30, G31, G32, M48, M401, M402. The functions important for leveling and homing are getting more simplified and unified also:

raise_z_after_probing       - always raises above the current position
raise_z_for_servo           - raise Z to the given offset, but only if needed
dock_sled                   - dock or undock a sled – just the moves. Called by `deploy`/`stow_z_probe`.
deploy_z_probe              - deploy the probe, undock the sled, etc. (raise for servo if needed)
stow_z_probe                - stow the probe, dock the sled, etc. (raise for servo if needed)
run_z_probe                 - probe the bed once, return with Z at the trigger position
set_bed_level_equation_lsq  - for grid leveling – non-delta only
set_bed_level_equation_3pts - for 3-point leveling (i.e., non grid leveling)
probe_pt                    - Raise Z, move to XY, (deploy), `run_z_probe`, (stow), return Z
homeaxis                    - home a single axis. For Z it may deploy/stow a probe.

I've made a couple of changes that seem to make sense, but I may need some feedback. Specifically, several bare calls to sync_plan_position seemed like they should be calling sync_plan_position_delta for delta.

I also split up these functions and call them depending on whether homing or probing…

setup_for_endstop_or_probe_move      - save feedrates, refresh the command timeout
setup_for_endstop_move               - as above, and enable endstops
clean_up_after_endstop_or_probe_move - restore feedrates, refresh the command timeout
clean_up_after_endstop_move          - as above, and set endstops to global state

I notice that in G28 it will call endstops.enable() even for homing of Z, even though Z homing might actually be done with a bed probe. The deploy function calls endstops.enable_z_probe() so the probe will be ready, so this isn't a big deal. Enabling endstops is probably harmless here.

[Roxy-3D]

OK! I'll review each of those in the morning. Do you have an estimate when a Z-Probe Endstop will work without Auto Bed Leveling turned on? It is enough work to merge with the RC-6 da jour that I want to plan my work so that I can avoid unecessary merge's. But as soon as a Z-Probe Endstop can be used with out Auto Bed Leveling, I want to do another merge so we are in sync.

[thinkyhead]

@Roxy-3D You should already be able to do so, but #4021 (https://github.com/thinkyhead/Marlin/tree/rc_raise_z_for_servos) is the most complete cleanup, and the one I'm most "confident" in — meaning I'm sure if anything was missed it will be pretty obvious. The DEBUG_LEVELING_FEATURE option will definitely help. Debug messages have been updated as part of the cleanup. Too bad we don't have an easy way to test SCARA robots at this time.

[Roxy-3D]

OK... I'll remerge in the morning and continue on...

[jbrazio]

@Roxy-3D can we close this one ?

[Roxy-3D]

Yes...