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

Raise the servo probe before stow outside ABL context #3942

Merged
merged 1 commit into from
Jun 3, 2016
Merged

Raise the servo probe before stow outside ABL context #3942

merged 1 commit into from
Jun 3, 2016

Conversation

lrpirlet
Copy link
Contributor

@lrpirlet lrpirlet commented Jun 1, 2016

This PR should ensure that the Z probe on a servo does raise before retract when used outside the ABL context.
Tested both within ABL context ( ABL enabled) and ouside ABL context (ABL disabled).

@thinkyhead
Copy link
Member

thinkyhead commented Jun 2, 2016

Where in the (original) code is the raise before servo stow done when ABL is enabled? I'm unable to locate the specific line.

@AnHardt
Copy link
Member

AnHardt commented Jun 2, 2016

Haha. Fooled by your macros?

    // Put away the Z probe
    #if ENABLED(Z_PROBE_SLED) || SERVO_LEVELING || ENABLED(FIX_MOUNTED_PROBE)
      if (axis == Z_AXIS && axis_home_dir < 0) {
        #if ENABLED(DEBUG_LEVELING_FEATURE)
          if (DEBUGGING(LEVELING)) SERIAL_ECHOLNPGM("> SERVO_LEVELING > " STRINGIFY(_Z_STOW));
        #endif
        _Z_STOW; // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
      }
    #endif
      #define _Z_STOW             (dock_sled(true))
    #elif SERVO_LEVELING || ENABLED(FIX_MOUNTED_PROBE)
      #define _Z_SERVO_TEST       (axis != Z_AXIS)      // servo.move XY
      #define _Z_PROBE_SUBTEST    false                 // Z will never be invoked
      #define _Z_DEPLOY           (deploy_z_probe())
      #define _Z_STOW             (stow_z_probe())  // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    #elif HAS_SERVO_ENDSTOPS
      #define _Z_SERVO_TEST       true                  // servo.move X, Y, Z
  static void stow_z_probe(bool doRaise = true) { // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    #if !(HAS_SERVO_ENDSTOPS && (Z_RAISE_AFTER_PROBING > 0))
      UNUSED(doRaise);
    #endif
...
    #if HAS_SERVO_ENDSTOPS

      // Retract Z Servo endstop if enabled
      if (servo_endstop_id[Z_AXIS] >= 0) {

        #if Z_RAISE_AFTER_PROBING > 0
          if (doRaise) {
            #if ENABLED(DEBUG_LEVELING_FEATURE)
              if (DEBUGGING(LEVELING)) {
                SERIAL_ECHOPAIR("Raise Z (after) by ", Z_RAISE_AFTER_PROBING);
                SERIAL_EOL;
                SERIAL_ECHO("> SERVO_ENDSTOPS > raise_z_after_probing()");
                SERIAL_EOL;
              }
            #endif
            raise_z_after_probing(); // this also updates current_position !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
            stepper.synchronize();
          }
        #endif

        // Change the Z servo angle
        servo[servo_endstop_id[Z_AXIS]].move(servo_endstop_angle[Z_AXIS][1]);
      }
  inline void raise_z_after_probing() {
    do_blocking_move_to_z(current_position[Z_AXIS] + Z_RAISE_AFTER_PROBING); // !!!!!!!!!!!!!!
  }

@lrpirlet
Copy link
Contributor Author

lrpirlet commented Jun 2, 2016

@AnHardt Thanks, I could not find it either, I only experienced it...
@thinkyhead Well, I have found that a servo probe DOES deploy outside ABL, DOES act as a Z endstop, Does retract when G28 finishes... This perfectly answer my needs and allows me to rely on a "moving probe", not on a static switch that needs adjustement anytime I change the glass, whatever is the room temperature...

The thing I designed and printed is nothing but a surface mount switch that the servo places in between the bed and the extruder tip...

Advantages? a Z-offset and NO X nor Y offset... nearly free if you care to dismount old mouses (you still have to invest time...) absolutely reliable if you think the number of click per day for a mouse... Mouses seldom die because of a switch failure, the cable is perfect for endstop signal and if USB the cable is even twisted pair... I like mouses :-)

@lrpirlet
Copy link
Contributor Author

lrpirlet commented Jun 2, 2016

Now, we can discuss what is the bug??? the fact that the servo deploys outside ABL context OR the fact that it retract without first raising..

I prefer to think that retract without raising is the bug... It fits better my needs when I decide that I do NOT need probing the bed with MBL.

@thinkyhead
Copy link
Member

thinkyhead commented Jun 2, 2016

Thanks @AnHardt. Yes, to some extent I was/am confused by my own macros! Especially since HAS_SERVO_ENDSTOPS will always be set when SERVO_LEVELING is set. So the test for HAS_SERVO_ENDSTOPS when stowing in homeaxis counts for both. Even though it's redundant, for clarity I should change the test to #if ENABLED(SERVO_LEVELING) || ENABLED(HAS_SERVO_ENDSTOPS).

@thinkyhead
Copy link
Member

thinkyhead commented Jun 2, 2016

the fact that the servo deploys outside ABL context OR the fact that it retract without first raising

@lrpirlet Well, we certainly do want a Z servo to deploy in homeaxis(Z_AXIS) when it's the only Z endstop on the machine, whether ABL is enabled or not. (P.S. When you said "MBL" above did you mean "ABL"?) And whenever it deploys or stows, we should have a raise. A default raise is a good idea.

But ultimately we need to make these raise/lower items available as general probing (including homing) options, and not dependent on ABL. Like we recently did with SAFE_HOMING. That's not too difficult, just a change to the configurations, and then scanning the G28 and homeaxis code. (Probably it means we can get rid of all references to ABL in the G28 and homeaxis routines.)

@thinkyhead
Copy link
Member

Take a look at #3946, which adds onto this PR with some of the changes we've discussed here.

@thinkyhead thinkyhead added the S: Don't Merge Work in progress or under discussion. label Jun 3, 2016
@lrpirlet
Copy link
Contributor Author

lrpirlet commented Jun 3, 2016

@thinkyhead Impressive... that amount of change would have taken me weeks a few months back, several days now... You did it in less than 24 hours...

That said, I do appreciate the changes made in #3946 and will test it sometime in the week-end...

To anser your question, I really mean "MY" Motorized Mesh Bed Leveling. I copied @epatel code that drives his Z switch palced over the hot tip... Only I designed a thing that, monted on a Z servo, moves a tiny SMB switch between the tip and the bed.

If interrested have a look at this fully funtional branch https://github.com/lrpirlet/Marlin/tree/motorized_mesh_bed_leveling_for_LRP

My goal now is to do that work again over an RCBugFix based branch modified with the LIN_ADVANCE algorithm.

@thinkyhead thinkyhead merged commit 3aefa04 into MarlinFirmware:RCBugFix Jun 3, 2016
@thinkyhead
Copy link
Member

thinkyhead commented Jun 4, 2016

You did it in less than 24 hours...

That's why I get paid the big bucks. Or, rather… the big thumbs ups.

a tiny SMB switch between the tip and the bed

Sounds good @lrpirlet !

My goal now is to do that work again

I've been keeping the LIN_ADVANCE branch (and PR) up to date with the latest code. I suggest starting with that. #3676

@lrpirlet lrpirlet deleted the G28_Z_servo_should_raise_before_stow branch June 5, 2016 11:57
@lrpirlet
Copy link
Contributor Author

lrpirlet commented Jun 5, 2016

@thinkyhead Thanks for that reference about your LIN_ADVANCE branch, I allready knew about it... and cloned it... Thanks also for the work to keep it current...

@thinkyhead thinkyhead mentioned this pull request Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Confirmed ! PR: Bug Fix S: Don't Merge Work in progress or under discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants