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

Fix Z_RAISE_AFTER_PROBING for non DELTA printers #3057

Merged

Conversation

jbrazio
Copy link
Contributor

@jbrazio jbrazio commented Feb 29, 2016

Discussed at #3034: Z_RAISE_AFTER_PROBING value was being ignored by Marlin because raise_z_after_probing() was only called if the printer type was set to a DELTA or if the printer had a docking sled for the probe.

raise_z_after_probing() was only called if the printer type was set to a
DELTA or if the printer had a docking sled for the probe.
@Blue-Marlin
Copy link
Contributor

This would rise twice for servo probes.

@jbrazio
Copy link
Contributor Author

jbrazio commented Feb 29, 2016

Is it related with this ?

        current_position[Z_AXIS] = -zprobe_zoffset + (z_tmp - real_z)
          #if HAS_SERVO_ENDSTOPS || ENABLED(Z_PROBE_ALLEN_KEY) || ENABLED(Z_PROBE_SLED)
             + Z_RAISE_AFTER_PROBING
          #endif
          ;

@Blue-Marlin
Copy link
Contributor

No. It's done much earlier in probe_pt() when called with probe_action = ProbeStow, stow_z_probe() is called, calling `raise_z_after_probing()'.

@jbrazio
Copy link
Contributor Author

jbrazio commented Feb 29, 2016

You're correct. I did pass by it because I was assuming HAS_SERVO_ENDSTOPS was associated with the sled, but by looking at Conditionals.h I now see clearly it's not.

    #if X_ENDSTOP_SERVO_NR >= 0 || Y_ENDSTOP_SERVO_NR >= 0 || Z_ENDSTOP_SERVO_NR >= 0
      #define HAS_SERVO_ENDSTOPS true

So would it suffice to change the patch from:

      // Sled assembly for Cartesian bots
      #if ENABLED(Z_PROBE_SLED)
        dock_sled(true); // dock the sled
      #elif Z_RAISE_AFTER_PROBING > 0
        raise_z_after_probing();
      #endif

To:

      // Sled assembly for Cartesian bots
      #if ENABLED(Z_PROBE_SLED)
        dock_sled(true); // dock the sled
      #elif !defined(HAS_SERVO_ENDSTOPS) && Z_RAISE_AFTER_PROBING > 0
        raise_z_after_probing();
      #endif

Or revert it and do something like:

      // Sled assembly for Cartesian bots
      #if ENABLED(Z_PROBE_SLED)
        dock_sled(true); // dock the sled
      #endif

      // Raise the probe
      #if !defined(HAS_SERVO_ENDSTOPS) && DISABLED(Z_PROBE_ALLEN_KEY) && DISABLED(Z_PROBE_SLED)
        raise_z_after_probing();
      #endif

Edit: Maybe DISABLED(Z_PROBE_ALLEN_KEY) is an overkill because at this point in the code we already know it's not a DELTA.

@jbrazio
Copy link
Contributor Author

jbrazio commented Mar 1, 2016

Ended up going with a third option which is a mix between the previous two, first check if the value of Z_RAISE_AFTER_PROBING is non-zero and only then exclude all the combinations where the Z axis could already have been raised i.e. stow_z_probe().

@thinkyhead
Copy link
Member

@jbrazio Do you mind making an equivalent patch for dev?

thinkyhead added a commit that referenced this pull request Mar 2, 2016
Fix Z_RAISE_AFTER_PROBING for non DELTA printers
@thinkyhead thinkyhead merged commit 186629a into MarlinFirmware:RCBugFix Mar 2, 2016
@jbrazio
Copy link
Contributor Author

jbrazio commented Mar 2, 2016

No problem, I'll submit a PR to it.

@jbrazio
Copy link
Contributor Author

jbrazio commented Mar 2, 2016

@thinkyhead I suggest for you to close #3034

@thinkyhead
Copy link
Member

@jbrazio Thanks! Done!

@jbrazio jbrazio deleted the 3034-z_raise_after_probing_fix branch March 3, 2016 11:47
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 18, 2016
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.

3 participants