Make probes independent of leveling

[thinkyhead]

This PR makes probing independent of leveling, so you can use your Z probe for homing even if you don't have bed leveling enabled.

Already merged in separate PRs:

• Move all deploy/stow handling into deploy_z_probe / stow_z_probe.
• Remove support for XY servo endstops (never properly supported anyway).
• Make zprobe_zoffset dependent only on HAS_BED_PROBE.
• Make M851 standard with no option to remap it, and only based on HAS_BED_PROBE.
• Enable probing GCodes and probe-homing based only on HAS_BED_PROBE.
• Simplify stowing code at the end of gcode_G29. (Over-simplified?)
• Sanity check Allen Key Probe for grid leveling only if leveling is enabled.

And…

• Initialize current_position to home_offset on boot
• Show uncorrected position in set_bed_level_equation_3pts
• Comment on run_z_probe
• Never do deploy-and-stow in probe_pt for Allen Key or Sled probes
• Split up endstop_move functions for probes / endstops
• Move deploy_z_probe after the setup_for_probe_or_endstop_move call in G29
• Use sync_plan_position_delta where needed using a macro

[Roxy-3D]

I think we need to do this. But I have a question about whether it actually fixes the G28 Z case with a servo. At the tail end of G28, we have this code:

    // Retract Servo endstop if enabled
    #if HAS_SERVO_ENDSTOPS
      if (_Z_SERVO_TEST && servo_endstop_id[axis] >= 0) {
        #if ENABLED(DEBUG_LEVELING_FEATURE)
          if (DEBUGGING(LEVELING)) SERIAL_ECHOLNPGM("> SERVO_ENDSTOPS > Stow with servo.move()");
        #endif
        servo[servo_endstop_id[axis]].move(servo_endstop_angle[axis][1]);
        if (_Z_PROBE_SUBTEST) endstops.enable_z_probe(false);
      }
    #endif

The code is going straight to the servo and not calling the stow_z_probe() function. Don't we need to change it to call stow_z_probe() to get the desired effect? Or alternatively, before this code messes with the servo, we need to call raise_z_after_probing() before it messes with the servo?

[thinkyhead]

That code should only apply to XY servos in the case where you have a Z probe. The code above that block will have already called stow_z_probe in the case of a Z probe.

// Put away the Z probe with a function
#if ENABLED(Z_PROBE_SLED) || SERVO_LEVELING || ENABLED(FIX_MOUNTED_PROBE)
  if (axis == Z_AXIS && axis_home_dir < 0) {
    stow_z_probe();
  }
#endif

For a servo-based Z probe, the second block ends up being reduced to…

// Retract X, Y (or Z) Servo endstop if enabled
#if ENABLED(HAS_SERVO_ENDSTOPS)
  if (axis != Z_AXIS && SERVO_ENDSTOP_EXISTS(axis)) {
    STOW_SERVO_ENDSTOP(axis);
  }
#endif // HAS_SERVO_ENDSTOPS

[Roxy-3D]

I see what you are saying... But I put the raise_z_after_probing() call in that X&Y code and it fixed the problem. My guess is it fell through that code first, did what I needed, and then later went through the Z code that was still wrong.

Wow!

[thinkyhead]

I'm unsure. With your addition, it should have reduced to…

// Retract Servo endstop if enabled
#if HAS_SERVO_ENDSTOPS
  if (axis != Z_AXIS && servo_endstop_id[axis] >= 0) {
    raise_z_after_probing();
    STOW_SERVO_ENDSTOP(axis);
  }
#endif

…which would only run if you have an index set for X or Y servos. Did you leave these commented out?:

//#define X_ENDSTOP_SERVO_NR 1
//#define Y_ENDSTOP_SERVO_NR 2

[Blue-Marlin]

It seems to be the right time to (remind) you that SERVO_ENDSTOPS can be dropped at all if the probes would always be usable in G28. They are only defined when bed leveling is defined. That's a conceptual error.

[Roxy-3D]

@Blue-Marlin Can you expand a little bit on that thought? It isn't quite coming across. But from what I did understand, I think I would agree if a probe is defined, it should be usable within the G28 command. I also think that G28 should not be concerned with how a probe does it's work. If the probe is implemented with a servo that work should be done at a lower level.

[Blue-Marlin]

Two stubs from homeaxis():
https://github.com/MarlinFirmware/Marlin/blob/RCBugFix/Marlin/Marlin_main.cpp#L2284-L2302
https://github.com/MarlinFirmware/Marlin/blob/RCBugFix/Marlin/Marlin_main.cpp#L2416-L2451
In both of them the probe block (including SERVO_LEVELING what is the servo probe) is follower by the HAS_SERVO_ENDSTOPS block. That is necessary, because the probes are not defined without bed levelling. If the probes would be defined outside AUTO_BED_LEVELING_FEATURE
https://github.com/MarlinFirmware/Marlin/blob/RCBugFix/Marlin/Configuration.h#L553
https://github.com/MarlinFirmware/Marlin/blob/RCBugFix/Marlin/Configuration.h#L596-647
we could always use them for homing.
The same nesting exists in Marlin_main.cpp

What is done in HAS_SERVO_ENDSTOPS is (should be) just a rewrite of the probe deploy/stow code for the case the probes are absent.
The advantage of eliminating the HAS_SERVO_ENDSTOPS-block and using the probes instead would be that all probes can be used for homing then. Ok, all probes you can deploy without knowing z - or you can deploy with not more risk then servo probes.

[Roxy-3D]

Thank You for the explanation. This sounds like something that should not be done prior to the Release Candidate going 'Golden'. But we should try to do this as soon as we can after that happens.

[thinkyhead]

I'm not sure if we're all clear about these flags. So a quick review…

• HAS_SERVO_ENDSTOPS is set whenever there is an X, Y, or Z servo endstop.
• HAS_SERVO_ENDSTOPS will always be set when SERVO_LEVELING is set.
• SERVO_LEVELING == AUTO_BED_LEVELING_FEATURE && HAS_Z_ENDSTOP_SERVO.
• The first block handling SERVO_LEVELING takes care of your Z Endstop Servo (servo probe) (when AUTO_BED_LEVELING_FEATURE is enabled).
• The second block handling HAS_SERVO_ENDSTOPS takes care of your X and Y Endstop Servos.
  • If AUTO_BED_LEVELING_FEATURE is disabled, then this block also takes care of your Z Endstop Servo, if there is one.

Yep, Marlin supports X and Y Servo Endstops (and Z Servo Endstops that are just endstops).

[AnHardt]

No, it does not anymore. Take a look into Endstops::update(). There are relics in the servo angles array and some other places, but i don't expect them to work. By the way - they are completely useless and would only complicate the code.
Even if there would be working x/y servo endstops it would be time to drop them.

[thinkyhead]

Happy to see this code go, then.

And why do we still have a three-element array for endstop servo angles, if we will only ever care about Z? Does this mean we could get rid of the other two elements now?

#define SERVO_ENDSTOP_ANGLES {{0,0}, {0,0}, {70,0}}

…replaced with…

#define Z_SERVO_ANGLES {70,0}

[AnHardt]

Does this mean we could get rid of the other two elements now?

Yes.

[AnHardt]

For this extruder with a servo we need again two defined angles. Maybe we can reuse the array with an other meaning.
However, the servo setup could be considerably simplified. What means - not in RC.

[thinkyhead]

For this extruder with a servo we need again two defined angles. Maybe we can reuse the array with an other meaning.

No need to re-use. It can just have its own distinct 2-member array.

[thinkyhead]

Ok, I've inserted a commit removing support for the obsolete servo endstops, along with some sanity checks…

#elif defined(SERVO_ENDSTOP_ANGLES)
  #error "SERVO_ENDSTOP_ANGLES is deprecated. Use Z_SERVO_ANGLES instead."
#elif defined(X_ENDSTOP_SERVO_NR) || defined(Y_ENDSTOP_SERVO_NR)
  #error "X_ENDSTOP_SERVO_NR and Y_ENDSTOP_SERVO_NR are deprecated and should be removed."
#endif

And, now I've added some more commits to allow probing if you have a probe, not requiring ABL.

[thinkyhead]

The more I hear about other workarounds the more I like re-using endstop switches for simple inputs. On many machines, at least 2, if not 3 endstop plugs are unused, so making them into assignable push-buttons would be a cheap way to add up to 3 simple functions to your old RepRap.

I suppose I could even warm up to the idea of using the endstops (when not homing) for other functions also, although until the endstops are clear of obstruction they are useless. You could wire NC switches in series, NO in parallel, but again, the endstops still need to be unpressed.

[AnHardt]

@thinkyhead

MANUALLY_DEPLOYED_PROBE

I'm a bit afraid to say that's more a long therm project, because you usually immediately begin with the realization then. So in this case i'll try to say - yes a very good idea - we need it now. Maybe waiting will be easier for you then.

An other good thing would be a MANUAL_PROBE. The deploy ends at MESH_HOME_SEARCH_Z. The actual probing is done by the click-wheel of the panel and ends with a click. Pretty much like today's manual probing in mesh-bed-leveling.

Other projects could include
making Allen-Key-Probes fit for Cartesian's.
Modularisierung probes (check - sequence of moves - special action - move back sequence - check : all parts can be omitted)

Buttons / Endstops

You kept me busy today.

[thinkyhead]

You kept me busy today.

I can tell we are all hungry for simplification in this area!

[jbrazio]

I like @thinkyhead idea, a "endstop input" if it has a endstop connected it will trigger endstop_stuff() other wise it will trigger button_stuff(), dead simple with no PIN #undefs and other crazy stuff.

[Roxy-3D]

The more I hear about other workarounds the more I like re-using endstop switches for simple inputs. On many machines, at least 2, if not 3 endstop plugs are unused, so making them into assignable push-buttons would be a cheap way to add up to 3 simple functions to your old RepRap.

I think this will help a lot of people.

MANUALLY_DEPLOYED_PROBE
I'm a bit afraid to say that's more a long therm project, because you usually immediately begin with the realization then. So in this case i'll try to say - yes a very good idea - we need it now. Maybe waiting will be easier for you then.

I believe this is very low risk and very easy. If the MANUALLY_DEPLOYED_PROBE is enabled, we just wait for an lcd_clicked() to continue in both stow and deploy of a probe. Later, we could do the extra work to make it flexible and use the CONTINUE_FEATURE.

An other good thing would be a MANUAL_PROBE. The deploy ends at MESH_HOME_SEARCH_Z. The actual probing is done by the click-wheel of the panel and ends with a click. Pretty much like today's manual probing in mesh-bed-leveling.

Yes! And it is already coded and working in the Unified Bed Leveling code:

float use_encoder_wheel_to_measure_point() {
    G29_has_control_of_LCD_Panel++;
    while (!G29_lcd_clicked() ) {       // we need the loop to move the nozzle based on the encoder wheel here!
        idle();
        if ( G29_encoderDiff != 0) {
            float new_z;                    
            // We define a new variable so we can know ahead of time where we are trying to go.
            // The reason is we want G29_encoderDiff cleared so an interrupt can update it even before the move 
            // is complete.  (So the dial feels responsive to user)
            new_z = current_position[Z_AXIS]+((float) G29_encoderDiff)/100.0;
            G29_encoderDiff = 0;            
            do_blocking_move_to_z( new_z );
        }   
    }
    while ( G29_lcd_clicked() ) {       // debounce and wait
        idle();
    }
    G29_has_control_of_LCD_Panel--;
    return current_position[Z_AXIS];
}

[thinkyhead]

Modularisierung probes

Very much so. I see a possibility of merger, top-down at first, to add automated MBL, and to generalize Grid and 3-point leveling. (In the direction of Universal Bed Leveling.) Basically the G29 function can be reduced to…

• Enter the G29 procedure for Automated probing
• Parse arguments such as grid size
  • For mesh probing set the mesh size, initializing mbl
  • For grid probing, use the size for the procedure
• Get the probe ready (if you need to do it early, such as for a sled)
• Multiply X grid points by Y grid points for the loop size
• Loop through the points…
  • Ask the "grid object" for the XY of the indexed probe point
    • The grid object will manage zigzag of points
  • Probe the XY to get Z
  • Tell the grid object the Z for the loop index (or xy indexes, or xy points)
• Put away the probe, if needed.
• Tell the grid object to calculate, enable compensation, etc.

The mesh_bed_leveling object is a pretty good pattern to follow for the grid and three-point leveling objects, as far as how it acts as a data provider to a leveling procedure rather than doing any leveling actions itself. The code in G29 will become a lot easier to follow as these parts are offloaded into class methods.

[thinkyhead]

use_encoder_wheel_to_measure_point

👍

[Roxy-3D]

The mesh_bed_leveling object is a pretty good pattern to follow for the grid and three-point leveling objects, as far as how it acts as a data provider to a leveling procedure rather than doing any leveling actions itself. The code in G29 will become a lot easier to follow as these parts are offloaded into class methods.

Please hold off on doing anything with the Mesh object or adding 3-Point or Grid or Delta objects. What I've got cooking already embraces all of those. And all of the nested #ifdef's are gone.

[AnHardt]

@Roxy-3D
If we'd have a preview we wouldn't have to guess.

[thinkyhead]

There are enough conceptual changes (mainly in probe_pt taking an offset rather than a Z position) that this PR probably should get another test run. All the commands that apply… G28, G29, G30, G31, G32, M48, M401, M402… and any others you might think of. The code considers Z raises as being above the probe's Z trigger position, and in other places the Z raise has been made double-raise-proof.

[Roxy-3D]

@AnHardt

If we'd have a preview we wouldn't have to guess.

How about in a couple of weeks? Basically, it is a high resolution (15x15) Mesh system that allows 3-Point and Grid Based to tilt the Mesh. Right now, I'm working on the Mesh editor to make it easy to update and/or fine tune your Mesh's. And it doesn't force you to prepare a Mesh. If you have a sufficiently flat piece of glass, you can just tell it to zero the entire Mesh and perform a tilt based on 3-Point or Grid probing. (So it is backwards compatible).

I don't have everything working... And my coding style is kind of chaotic where I paste old functions and header files into my files just so I have easy reference. I have to do some clean up before I'm ready to share the code.

What I'm trying to do is stay synchronized with ThinkyHead so when we promote the Release Candidate to 'Golden' status, we have a Development Branch ready to go with Unified Bed Leveling based on the Stable Branch.

[AnHardt]

@thinkyhead
I'll try to test as much as i can, but i'v to prepare a birthday party an guests over the weekend.

@Roxy-3D
You are underestimating us - we are able read Marlin.

[Roxy-3D]

You underestimate us - we can read Marlin.

No. You are wrong. I'm not under estimating you. I know your code is clean and I don't want to share anything that is not just as clean.

[AnHardt]

@Roxy-3D

@)>--->--- (rose)

I guess @thinkyhead has an other opinion about my style.
(However 3:55 am - good night)

[thinkyhead]

This addresses enough bugs, logical errors, and especially compile issues, that I thought it best to merge it now. It's modular enough that we can backtrack on any section, should there be any unexpected behavior.

[AnHardt]

Tried some things we talked about yesterday AnHardt#48

[thinkyhead]

PR #4134 incorporates most of those changes, which are very sensible.